add GHA skeleton for routing regression pipeline by Sherley-Sonali · Pull Request #12 · valhalla/RAD

Sherley-Sonali · 2026-06-08T11:22:21Z

Adds .github/workflows/routing_regression.yml - skeleton for the routing regression pipeline .

Four jobs: build Valhalla from source with ccache via Hetzner S3, build graph tiles from the LFS PBF and upload as artifact, run route requests in parallel against old and new router via pyvalhalla, diff and store results in RAD-data.
Processing scripts (run_routes.py, diff_responses.py, push_results.py) are left as TODOs for a follow-up PR. Known gaps are documented inline.

closes #10

Used AI as a drafting/thinking aid for implementation and test design. All changes were reviewed, tested, and understood before submission.-

…ermutations

Sherley-Sonali · 2026-06-10T07:56:50Z

key decisions in this skeleton:

merged build-valhalla and build-tiles into one job - ccache via Hetzner S3 handles making repeat builds fast, no need to pass binaries between jobs
workflow_dispatch inputs drive all 4 permutations (old/new router × old/new graph) without code changes - just pass different refs at trigger time
tiles uploaded as 90-day GHA artifact, reused across runs - only rebuilt when rebuild_tiles=true
run-routes matrix parallelizes old and new router on separate machines simultaneously
processing scripts (run_routes.py, diff_responses.py, push_results.py) are TODOs for next PR

nilsnolde

thanks. I think for now you can ignore the comments. I needed to think about this a bit more and see what makes most sense.

IMO we have to split this up into multiple ymls. one for building tiles, one for routing regression tests, at least. so we can keep the layout sane.

you know what @Sherley-Sonali . best you research how to semantically do this best with multiple files so the layout makes most sense. keep some stuff in mind for research:

workflow_call does "reusable workflows" (could be e.g. the testing yml calling the build_tiles workflow)
we can have our own .github/workflows/actions/build-valhalla.yml which centralizes the build for both (and eventually most) workflows

nilsnolde · 2026-06-10T10:48:07Z

+      - name: Download tiles artifact
+        uses: actions/download-artifact@v4
+        with:
+          name: ${{ inputs.tiles_artifact }}


per your description of the input: valhalla-tiles-master will download what from where? we have nothing building a tileset for master yet.

I'm not quite sure how to handle this exactly.. artifacts work per workflow run, meaning you'd always need to build both "old" and "new" graphs in each run. if we use artifacts for this. let me think for a second.

this still stands @Sherley-Sonali, I still don't see how this could make sense with artifacts.

nothing here actually says "build tiles". there's apparently some GHA config for it, but who calls that?

…g-regression workflows

Sherley-Sonali · 2026-06-15T09:08:37Z

Here's a first pass at the layout split, based on the points you raised:

build-valhalla.yml - reusable workflow (workflow_call), takes a valhalla_ref input, builds Valhalla from source with ccache (S3-backed). This is the single place that owns the build/cmake config - both other workflows call into it.
build-tiles.yml - workflow_dispatch, calls build-valhalla, then builds tiles from OSM data and uploads them as an artifact. Run independently whenever OSM data/Mjolnir changes.
routing-regression.yml - workflow_dispatch, calls build-valhalla for an old/new ref pair, downloads a tiles artifact, runs the route diff. Replaces routing_regression.yml.

Existing TODOs (admins.sqlite, run_routes.py, diff_responses.py, push_results.py, RAD-data push race) carried over as-is - this PR is just the layout split, no new functionality yet.

nilsnolde · 2026-06-15T10:40:23Z

@Sherley-Sonali hm, seems I worded it wrong: of course a lot of my comments are still valid! and you need to address them! just not the ones which try to review logic which would be changing anyways with my suggestion to split it into multiple files.

nilsnolde

still some way to go. also keep in mind that eventually we need 2 refs to build tiles from, even if we just start with "old" for now.

nilsnolde · 2026-06-15T10:33:33Z

+    secrets: inherit
+
+  build-tiles:
+    runs-on: ubuntu-24.04


Suggested change

runs-on: ubuntu-24.04

runs-on: ubuntu-latest

we want to know early if there's anything failing with current releases

nilsnolde · 2026-06-15T10:35:18Z

+  build-valhalla:
+    uses: ./.github/workflows/build-valhalla.yml
+    with:
+      valhalla_ref: ${{ inputs.valhalla_ref }}
+    secrets: inherit


seems like a redundant job..

build-valhalla now produces a wheel artifact that build-tiles downloads and installs, no rebuilding inline

nilsnolde · 2026-06-16T03:45:33Z

+      - name: Rebuild Valhalla
+        run: |
+          cmake -B valhalla-src/build -S valhalla-src \
+            -DCMAKE_BUILD_TYPE=Release \
+            -DENABLE_PYTHON_BINDINGS=ON \
+            -DENABLE_SERVICES=OFF \
+            -DENABLE_TESTS=OFF \
+            -DENABLE_BENCHMARKS=OFF \
+            -DENABLE_CCACHE=ON \
+            -DENABLE_TOOLS=OFF \
+            -DENABLE_GEOTIFF=OFF \
+            -DENABLE_LZ4=OFF
+          make -C valhalla-src/build -j$(nproc)
+          sudo make -C valhalla-src/build install


why do the build "inline" if there's a drop-in workflow call?

addressed in redesign

nilsnolde · 2026-06-16T03:51:25Z

+          valhalla_build_admins -c valhalla.json data/liechtenstein_graph.osm.pbf
+          valhalla_build_config \
+            --mjolnir-tile-dir valhalla_tiles > valhalla.json
+          valhalla_build_tiles -c valhalla.json data/liechtenstein_graph.osm.pbf


did you actually try this?

verified locally now

please keep in mind to always verify locally before you actually commit, or at least before you ask for review! it's part of the learning process.

nilsnolde · 2026-06-16T03:52:59Z

+      - name: Download tiles artifact
+        uses: actions/download-artifact@v4
+        with:
+          name: ${{ inputs.tiles_artifact }}


this still stands @Sherley-Sonali, I still don't see how this could make sense with artifacts.

nothing here actually says "build tiles". there's apparently some GHA config for it, but who calls that?

nilsnolde · 2026-06-16T03:53:24Z

+      - name: Build Valhalla at router ref
+        run: |
+          cmake -B valhalla-src/build -S valhalla-src \
+            -DCMAKE_BUILD_TYPE=Release \
+            -DENABLE_PYTHON_BINDINGS=ON \
+            -DENABLE_SERVICES=OFF \
+            -DENABLE_TESTS=OFF \
+            -DENABLE_BENCHMARKS=OFF \
+            -DENABLE_CCACHE=ON \
+            -DENABLE_TOOLS=OFF \
+            -DENABLE_GEOTIFF=OFF \
+            -DENABLE_LZ4=OFF
+          make -C valhalla-src/build -j$(nproc)
+          sudo make -C valhalla-src/build install


no inline build now

Sherley-Sonali · 2026-06-16T18:05:34Z

tiles are built by running build-tiles.yml separately and it uploads them as a 90-day artifact. routing-regression.yml takes tiles_run_id as input and uses gh run download to fetch that artifact cross-run.

And on needing - 2 refs to build tiles:
The new build-tiles.yml currently takes one valhalla_ref input. For the future 4 permutations (new router + new graph), you'd run build-tiles.yml twice with different refs. The artifact names valhalla-tiles-{ref_slug} already handle this and each ref produces a distinct artifact (just run it twice).

nilsnolde

so to summarize how it currently would work when I'd like to run a regression test some day:

I run build-tiles.yml manually with the right git SHAs, which builds the bindings for each SHA
I wait for that process to finish: AFAIK I have to refresh the PR page constantly, bcs I will not get notified from GH when the tile build run exited with success
then I manually run routing-regression.yml with the same input as build-tiles.yml plus a tiles_run_id, which I have to hunt down from the 1. step (build-tiles.yml run)
in routing-regression.yml, for each SHA we again build valhalla (as we already did for the build-tiles.yml step, but even more do I wonder why you sync wheels via artifacts in the first place), download the valhalla wheel & the graph from GH artifacts via a brittle tiles_run_id, then push the route responses before
diffing them in another job

everything before 5. is not an ergonomical workflow and very very humany error prone.

I see it this way: the routing-regression.yml is the only thing we ever need to manually run for a route regression test. everything else derives from this one source of truth, which orchestrates everything else. build-valhalla.yml is run exactly once per SHA, build-tiles.yml is run inline for each SHA in the route regression workflow.

nilsnolde · 2026-06-17T09:29:23Z

+          pip wheel . --no-build-isolation --wheel-dir /tmp/valhalla-dist \
+            -Ccmake.build-type=Release \
+            -Ccmake.define.ENABLE_PYTHON_BINDINGS=ON \
+            -Ccmake.define.ENABLE_TESTS=OFF \
+            -Ccmake.define.ENABLE_SERVICES=OFF


I don't think this'll currently work well for caching. ccache uses a lot of heuristics to invalidate cache hits and I'm quite sure compilation commands might use e.g. -I </tmp/pip-build-xxx> absolute paths which would trigger invalidation. that is bcs pip wheel uses /tmp to build the wheel.

this is just an educated guess. can you make sure that doesn't happen currently by simply executing this command twice on your local machine (of course with ccache installed)? you'll need to watch ccache hits/stats before & after the second run.

and what happened to #12 (comment)? the way you use it now would need ENABLE_TOOLS=ON but the others are still not necessary.

nilsnolde · 2026-06-17T09:30:45Z

+      - name: Install Python build dependencies
+        run: pip install scikit-build-core pyproject-metadata setuptools-scm pybind11
+


this is pretty bad for maintenance: whenever we add a build dependency, we now have to update two places! remove --build-isolation and this step

nilsnolde · 2026-06-17T09:32:41Z

+          valhalla_build_admins -c valhalla.json data/liechtenstein_graph.osm.pbf
+          valhalla_build_config \
+            --mjolnir-tile-dir valhalla_tiles > valhalla.json
+          valhalla_build_tiles -c valhalla.json data/liechtenstein_graph.osm.pbf


please keep in mind to always verify locally before you actually commit, or at least before you ask for review! it's part of the learning process.

nilsnolde · 2026-06-17T09:33:20Z

+      - name: Install system dependencies
+        run: bash valhalla-src/scripts/install-linux-deps.sh


first install, then restore cache

nilsnolde · 2026-06-17T09:35:44Z

+      - name: Restore ccache
+        uses: tespkg/actions-cache/restore@v1
+        with:
+          endpoint: ${{ secrets.HETZNER_S3_ENDPOINT }}


where did you get this secret from?

nilsnolde · 2026-06-17T12:38:13Z

+      - name: Install Valhalla from wheel
+        run: pip install /tmp/valhalla-dist/*.whl
+


we do want to control the python version this runs with.

nilsnolde · 2026-06-17T12:42:35Z

+      - name: Download tiles from build-tiles run
+        env:
+          GH_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+        run: |
+          gh run download ${{ inputs.tiles_run_id }} \
+            --repo "${{ github.repository }}" \
+            --pattern "valhalla-tiles-*" \
+            --dir valhalla_tiles
+          shopt -s dotglob
+          mv valhalla_tiles/valhalla-tiles-*/* valhalla_tiles/ 2>/dev/null || true


it's not priority, but worth mentioning: one thing we said we want to look out for is future compatibility with e.g. scenario old graph/new graph. currently it's fixed to using a single graph, whatever tiles_run_id references. could do 2 of those, but then I'd need to look up 4 things: old/new SHA, old/new run_id. all separately. this is super duper error prone and can really really take time to realize, when smth nasty seems to happen on the diffs we kick off. avoid at all costs!

don't try hard to keep that requirement in mind for the next round of edits. we can deal with more scenarios once we get there. I just needed to mention it.

also don't sync via artifacts. let the graphs be uploaded to S3, that's a much more idiomatic place for them, especially the master graph. PR graphs should be uploaded to S3 as well, they'll be cleaned once a PR closes.

nilsnolde · 2026-06-17T12:49:15Z

+          valhalla_build_config \
+            --mjolnir-tile-dir "$(pwd)/valhalla_tiles" \
+            > /tmp/valhalla.json


pass the dir (after your next edit: the tar of the dir) to run_routes.py instead. pyvalhalla can deal with simply the path, no need for an external config!

nilsnolde · 2026-06-17T12:50:12Z

+      - name: Upload responses artifact
+        uses: actions/upload-artifact@v4
+        with:
+          name: responses-${{ matrix.router.name }}
+          path: /tmp/responses-${{ matrix.router.name }}.jsonl
+          retention-days: 7


this is literally the only thing we want uploaded to GH artifacts.

nilsnolde · 2026-06-17T13:08:37Z

+      tiles_run_id:
+        description: "Run ID from a successful build-tiles.yml run"
+        required: true


this is pretty brittle and awkward to hunt down.

Sherley-Sonali added 2 commits June 8, 2026 16:47

add GHA skeleton for routing regression pipeline

4bfc820

update GHA skeleton: merged build jobs, ccache via S3, matrix for 4 p…

9cbc9b8

…ermutations

nilsnolde requested changes Jun 10, 2026

View reviewed changes

ci: split routing_regression into build-valhalla, build-tiles, routin…

383db98

…g-regression workflows

addressed review comments

84867be

nilsnolde requested changes Jun 16, 2026

View reviewed changes

add run_routes and diff_responses scripts, update workflows

8635d36

nilsnolde requested changes Jun 17, 2026

View reviewed changes

		- name: Install Python build dependencies
		run: pip install scikit-build-core pyproject-metadata setuptools-scm pybind11

		- name: Install system dependencies
		run: bash valhalla-src/scripts/install-linux-deps.sh

		- name: Install Valhalla from wheel
		run: pip install /tmp/valhalla-dist/*.whl

Uh oh!

Conversation

Sherley-Sonali commented Jun 8, 2026

Uh oh!

Sherley-Sonali commented Jun 10, 2026

Uh oh!

nilsnolde left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Sherley-Sonali commented Jun 15, 2026

Uh oh!

nilsnolde commented Jun 15, 2026

Uh oh!

nilsnolde left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Sherley-Sonali commented Jun 16, 2026

Uh oh!

nilsnolde left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

nilsnolde left a comment •

edited

Loading