Bump actions to current majors, and make tsp_christofides actually work

.github/workflows/ci.yml

@@ -15,10 +15,10 @@ jobs:
   lint:
     runs-on: ubuntu-latest
     steps:
-    - uses: actions/checkout@v4
+    - uses: actions/checkout@v7
 
     - name: Install uv
-      uses: astral-sh/setup-uv@v7
+      uses: astral-sh/setup-uv@v9.0.0
       with:
         enable-cache: true
         cache-dependency-glob: "uv.lock"
@@ -57,10 +57,10 @@ jobs:
       matrix:
         python-version: ["3.11", "3.12", "3.13"]
     steps:
-    - uses: actions/checkout@v4
+    - uses: actions/checkout@v7
 
     - name: Install uv
-      uses: astral-sh/setup-uv@v7
+      uses: astral-sh/setup-uv@v9.0.0
       with:
         enable-cache: true
         cache-dependency-glob: "uv.lock"
@@ -77,7 +77,7 @@ jobs:
         uv run pytest tests/ --cov=allocator --cov-report=xml --cov-report=term-missing -v --tb=short
 
     - name: Upload coverage to Codecov
-      uses: codecov/codecov-action@v5
+      uses: codecov/codecov-action@v7
       if: matrix.python-version == '3.11'
       with:
         file: ./coverage.xml
@@ -87,10 +87,10 @@ jobs:
   test-algorithms:
     runs-on: ubuntu-latest
     steps:
-    - uses: actions/checkout@v4
+    - uses: actions/checkout@v7
 
     - name: Install uv
-      uses: astral-sh/setup-uv@v7
+      uses: astral-sh/setup-uv@v9.0.0
       with:
         enable-cache: true
         cache-dependency-glob: "uv.lock"
@@ -104,7 +104,7 @@ jobs:
 
     - name: Test advanced algorithms
       run: |
-        uv run pytest tests/api/ -k "ortools or google" -v --tb=short
+        uv run pytest tests/api/ -k "ortools or google or christofides" -v --tb=short
 
   # Every other job installs from uv.lock, so CI and a developer's machine agree.
   # That is the point, and it has a cost: CI stops noticing when a new release
@@ -120,19 +120,17 @@ jobs:
     runs-on: ubuntu-latest
     if: github.event_name == 'schedule' || github.event_name == 'workflow_dispatch'
     steps:
-    - uses: actions/checkout@v4
+    - uses: actions/checkout@v7
 
     - name: Install uv
-      uses: astral-sh/setup-uv@v7
+      uses: astral-sh/setup-uv@v9.0.0
 
     - name: Set up Python 3.11
       run: uv python install 3.11
 
-    # The same extras the lint and test jobs use, deliberately not --all-extras:
-    # tests/api/test_route_api.py::test_christofides_import_error_handling
-    # asserts that tsp_christofides *raises* ImportError, so installing the
-    # algorithms extra here would fail it for reasons that have nothing to do
-    # with upstream drift.
+    # The same extras the lint and test jobs use, deliberately not --all-extras,
+    # so this job checks the install most people actually have rather than the
+    # maximal one.
     - name: Resolve the latest of everything, ignoring the lock
       run: uv sync --upgrade --extra dev --extra test --all-groups
 
@@ -152,10 +150,10 @@ jobs:
     runs-on: ubuntu-latest
     needs: [lint, test]
     steps:
-    - uses: actions/checkout@v4
+    - uses: actions/checkout@v7
 
     - name: Install uv  
-      uses: astral-sh/setup-uv@v7
+      uses: astral-sh/setup-uv@v9.0.0
 
     - name: Set up Python 3.11
       run: |

.github/workflows/docs.yml

@@ -19,10 +19,10 @@ jobs:
   build:
     runs-on: ubuntu-latest
     steps:
-    - uses: actions/checkout@v4
+    - uses: actions/checkout@v7
 
     - name: Install uv
-      uses: astral-sh/setup-uv@v7
+      uses: astral-sh/setup-uv@v9.0.0
       with:
         enable-cache: true
         cache-dependency-glob: "uv.lock"
@@ -49,12 +49,17 @@ jobs:
         touch build/html/.nojekyll
 
     - name: Setup Pages
-      uses: actions/configure-pages@v5
+      uses: actions/configure-pages@v6
 
     - name: Upload artifact
-      uses: actions/upload-pages-artifact@v3
+      uses: actions/upload-pages-artifact@v5
       with:
         path: 'docs/build/html'
+        # v4 started excluding dotfiles from the artifact. The build step writes
+        # .nojekyll, and Sphinx output is full of _static and _sources, so losing
+        # it is the kind of change that shows up as a broken site rather than a
+        # failed job.
+        include-hidden-files: true
 
   deploy:
     environment:
@@ -66,4 +71,4 @@ jobs:
     steps:
     - name: Deploy to GitHub Pages
       id: deployment
-      uses: actions/deploy-pages@v4
+      uses: actions/deploy-pages@v5

.github/workflows/python-publish.yml

@@ -21,10 +21,10 @@ jobs:
   test-before-publish:
     runs-on: ubuntu-latest
     steps:
-    - uses: actions/checkout@v4
+    - uses: actions/checkout@v7
 
     - name: Install uv
-      uses: astral-sh/setup-uv@v7
+      uses: astral-sh/setup-uv@v9.0.0
       with:
         enable-cache: true
         cache-dependency-glob: "uv.lock"
@@ -48,12 +48,12 @@ jobs:
       url: ${{ github.event.inputs.use_test_pypi == 'true' && 'https://test.pypi.org/p/allocator' || 'https://pypi.org/p/allocator' }}
 
     steps:
-    - uses: actions/checkout@v4
+    - uses: actions/checkout@v7
       with:
         fetch-depth: 0
 
     - name: Install uv
-      uses: astral-sh/setup-uv@v7
+      uses: astral-sh/setup-uv@v9.0.0
 
     - name: Set up Python 3.11
       run: |

allocator/core/routing.py

@@ -99,13 +99,15 @@ def solve_tsp_christofides(
     Returns:
         (total_distance, route) tuple
     """
-    try:
-        from Christofides import christofides
-    except ImportError as e:
-        raise ImportError(
-            "Christofides algorithm requires the 'Christofides' package. "
-            "Install it with: pip install Christofides"
-        ) from e
+    # networkx's own approximation, not the Christofides package on PyPI. That
+    # package is Python 2 source -- importing it raises SyntaxError on `print
+    # 'Testing...'` -- so this function had never once run on Python 3. Nothing
+    # noticed because the only test asserted the package was *absent*, and the
+    # CI job that installed it filtered the christofides tests out.
+    #
+    # networkx is already a hard dependency and has provided this since 2.6, so
+    # there is no optional import to guard.
+    from networkx.algorithms.approximation import christofides
 
     # Get distance matrix
     distances = get_distance_matrix(points, points, method=distance_method, **distance_kwargs)
@@ -121,15 +123,20 @@ def solve_tsp_christofides(
         for j in range(i + 1, n):
             G.add_edge(i, j, weight=distances[i, j])
 
-    # Solve using Christofides algorithm
-    tour = christofides(G, 0)  # Start from node 0
+    # A Hamiltonian cycle, so the last node repeats the first.
+    cycle = christofides(G, weight="weight")
 
-    # Calculate total distance
+    # Length of the closed tour, including the leg back to the start.
     total_distance = 0.0
-    for i in range(len(tour) - 1):
-        total_distance += distances[tour[i], tour[i + 1]]
-
-    return total_distance, tour
+    for i in range(len(cycle) - 1):
+        total_distance += distances[cycle[i], cycle[i + 1]]
+
+    # Return the closed cycle, first node repeated at the end, matching
+    # solve_tsp_ortools. The two solvers are interchangeable through
+    # shortest_path(), so a caller switching methods must not silently get a
+    # different route convention -- and tests/api/test_route_api.py pins the
+    # ortools one at len(points) + 1.
+    return float(total_distance), list(cycle)
 
 
 def solve_tsp_osrm(

tests/api/test_route_api.py

@@ -2,13 +2,15 @@
 Tests for the modern routing API.
 """
 
+import itertools
 import unittest
 
 import numpy as np
 import pandas as pd
 
 from allocator.api import shortest_path, tsp_christofides, tsp_ortools
 from allocator.api.types import RouteResult
+from allocator.distances import get_distance_matrix
 
 
 class TestRouteAPI(unittest.TestCase):
@@ -84,12 +86,70 @@ def test_high_level_shortest_path_function(self):
         except ImportError:
             self.skipTest("OR-Tools not available")
 
-    def test_christofides_import_error_handling(self):
-        """Test Christofides handles missing dependencies gracefully."""
-        with self.assertRaises(ImportError) as cm:
-            tsp_christofides(self.test_points)
+    def test_christofides_solves_a_tour(self):
+        """Christofides returns a tour that visits every point exactly once.
+
+        This replaces ``test_christofides_import_error_handling``, which asserted
+        that ``tsp_christofides`` *raises* ImportError. That only happens when the
+        ``algorithms`` extra is absent, so the test meant one thing in the plain
+        test job and the opposite in test-algorithms -- and it passed only
+        because nobody had installed the extra on that runner.
+
+        Running it the other way is what showed the function had never worked on
+        Python 3: the Christofides package on PyPI is Python 2 source and raises
+        SyntaxError on import. It now uses networkx's approximation, which is a
+        hard dependency already, so there is no optional import left to test.
+        """
+        result = tsp_christofides(self.test_points)
+        n_points = len(self.test_points)
+
+        self.assertIsInstance(result, RouteResult)
+        # Closed tour, matching tsp_ortools: every point once, then back to the
+        # start. The two solvers are interchangeable through shortest_path(), so
+        # they must agree on what a route is.
+        self.assertEqual(len(result.route), n_points + 1)
+        self.assertEqual(result.route[0], result.route[-1])
+        self.assertEqual(sorted(result.route[:-1]), list(range(n_points)))
+        self.assertGreater(result.total_distance, 0)
+
+    def test_christofides_stays_within_its_approximation_guarantee(self):
+        """The property that makes Christofides worth using over any other tour.
+
+        It is a 3/2-approximation on a metric instance, so on a problem small
+        enough to solve exactly the tour must be no worse than 1.5x optimal. A
+        merely "valid" tour -- every point once, positive length -- would also be
+        produced by visiting the points in input order, so without this the test
+        above does not distinguish the algorithm from doing nothing.
+        """
+        result = tsp_christofides(self.test_points)
+
+        # Score the tour in the metric the solver actually optimised in.
+        # `euclidean` projects lon/lat to UTM metres before measuring
+        # (allocator/distances/euclidean.py, utm.from_latlon), so scoring with
+        # distances computed from raw degrees would compare a UTM-optimal tour
+        # against a degree-optimal one. That is a different problem, and its
+        # ratio can exceed 3/2 through projection distortion alone.
+        points = self.test_points[["longitude", "latitude"]].to_numpy()
+        n = len(points)
+        distances = get_distance_matrix(points, points, method="euclidean")
+
+        optimal = min(
+            sum(distances[order[i], order[i + 1]] for i in range(n - 1))
+            + distances[order[-1], order[0]]
+            for order in itertools.permutations(range(n))
+        )
+        # result.route is the closed tour; drop the repeated start to iterate.
+        route = result.route[:-1]
+        tour = (
+            sum(distances[route[i], route[i + 1]] for i in range(n - 1))
+            + distances[route[-1], route[0]]
+        )
 
-        self.assertIn("Christofides", str(cm.exception))
+        self.assertLessEqual(
+            tour,
+            1.5 * optimal,
+            f"tour {tour:.4f} exceeds 1.5x the optimum {optimal:.4f}",
+        )
 
     def test_invalid_method(self):
         """Test error handling for invalid TSP method."""