From 833841f9de44b2b6a1fe8c918f785cf6bb916062 Mon Sep 17 00:00:00 2001 From: Jaak Laineste Date: Wed, 9 Sep 2026 09:21:13 +0000 Subject: [PATCH] ci(example): make the artifact-currency check advisory on pull requests The worked example regenerates from three live Estonian services, and the final step diffs the regenerated project.qgz against the committed one. That step answers "has the committed example stopped describing its sources?" -- a question about the world, not about a branch. Its legend embeds facility counts, so one kindergarten opening in Tartu rewrites the file and blocks a merge that has nothing to do with it. That is what happened on 2026-09-09: PR #25 changed only the CLI, and the job went red because the upstream parcel count moved 518 -> 517. The same step had already failed on main's scheduled run two days earlier, which is the control -- the drift preceded the branch. So scope the assertion to the event rather than dropping it. On the schedule and on workflow_dispatch it stays a hard failure: that run exists to notice drift. On a pull request it reports a warning and does not block. The steps a PR is genuinely gated on are unchanged -- validate, verify, and the real-QGIS render all still ask whether the change broke the example. This is the workflow's own stated intent ("an outage upstream must not redden unrelated work") applied to data drift as well as to outages. Note that main stays red on the schedule until someone with access to the live sources reruns examples/tartu-development/pipeline.py and commits the regenerated project.qgz and run record. That is the alarm working. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01DqoLGyKY5opHGjNMSerpHg --- .github/workflows/example.yml | 24 +++++++++++++++++++++--- docs/maintainers/debugging.md | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 56 insertions(+), 3 deletions(-) diff --git a/.github/workflows/example.yml b/.github/workflows/example.yml index 6afd1c4..de34356 100644 --- a/.github/workflows/example.yml +++ b/.github/workflows/example.yml @@ -9,7 +9,10 @@ name: Worked example (full validation) # # It reaches three external services (Maa- ja Ruumiamet S3, the ETAK WFS, and # Tartu's ArcGIS Feature Services), so it is deliberately not on every PR: -# an outage upstream must not redden unrelated work. +# an outage upstream must not redden unrelated work. For the same reason the +# artifact-currency check at the end is advisory on a pull request and a hard +# failure on the schedule -- live sources drift on their own timetable, which +# is a fact about the world rather than a defect in someone's branch. on: schedule: @@ -64,11 +67,26 @@ jobs: # facility counts, so an upstream change to Tartu's education data # will trip this too -- also correctly: the committed example has then # stopped describing the current sources and wants regenerating. + # + # That second case is why this is advisory on a pull request. The + # sources are live third-party services, so a kindergarten opening in + # Tartu changes project.qgz without anything in the PR being wrong, + # and the diff would then block a merge for a fact about the world. + # The scheduled run is the one whose job is to notice that drift, and + # there it stays a hard failure. The steps above -- validate, verify, + # and the real-QGIS render -- are what a PR is actually gated on: they + # ask whether this change broke the example, not whether the example + # still matches sources that moved underneath it. + continue-on-error: ${{ github.event_name == 'pull_request' }} + env: + EVENT_NAME: ${{ github.event_name }} run: | git diff --exit-code -- examples/tartu-development/project.qgz || { - echo "::error::The committed project.qgz differs from the one pipeline.py"\ + if [ "$EVENT_NAME" = "pull_request" ]; then level=warning; else level=error; fi + echo "::${level}::The committed project.qgz differs from the one pipeline.py"\ "just produced. Re-run examples/tartu-development/pipeline.py and"\ - "commit the regenerated project.qgz (and its run record)." + "commit the regenerated project.qgz (and its run record). On a pull"\ + "request this is advisory: upstream Tartu data moves on its own." git diff --stat -- examples/tartu-development/project.qgz exit 1 } diff --git a/docs/maintainers/debugging.md b/docs/maintainers/debugging.md index 6407bd8..b969c8e 100644 --- a/docs/maintainers/debugging.md +++ b/docs/maintainers/debugging.md @@ -122,6 +122,41 @@ Two related traps: - a relation's detection power depends on the data and the variant size. Widening the mini-Tartu road threshold by 1.5× cannot expose an inverted predicate because the only far parcel sits at 5450 m; the fixture declares `variant: {multiply: 3}` for that reason. When a mutation survives, check the geometry before suspecting the relation; - a GeoJSON output without a `crs` member reads back as EPSG:4326. A pipeline that writes analysis-CRS coordinates into plain GeoJSON and declares `EPSG:3301` in the manifest fails `geodata.dataset_crs_is` correctly. Write the `crs` member (or use GeoParquet) rather than relaxing the check. +## A red worked-example job is usually the world moving, not the branch + +`.github/workflows/example.yml` regenerates `examples/tartu-development/` from +live Estonian services, so its failures split into two kinds that look +identical in the checks list: + +- **the change broke the example** — reported by `validate`, `verify`, or the + real-QGIS render step; +- **the sources moved** — reported only by the final artifact-currency step, + which diffs the regenerated `project.qgz` against the committed one. The + legend embeds facility counts (`Verified municipal schools (n=26)`), so one + opening or closing in Tartu rewrites the file. It is advisory on a pull + request and a hard failure on the schedule for exactly this reason. + +To tell them apart without guessing, read the job's uploaded `worked-example-run` +artifact rather than re-running anything: its run record inventories every +output by SHA-256 and its validation report carries feature counts, both +comparable with what the repository ships. + +```bash +gh run download -n worked-example-run -D /tmp/wx +``` + +Observed 2026-09-09 (issue #20's PR): the counts read 517 against a committed +518, seven of the eight inventoried outputs differed, and `project.qgz` +differed at an identical 3698 bytes — consistent with a same-length digit +substitution in one of those legend counts. + +Note what that byte-identical size rules out. Comparing a `.qgz` compares a +deflate stream, so a zlib or QGIS change could in principle move the bytes with +the content unchanged. It was not the cause there and the container is +deterministic by construction — `write_qgis_project` pins the zip entry to a +1980 timestamp and fixed permissions — but a currency failure with *no* +accompanying digest drift in the run record is the signature to suspect. + ## Generated benchmark artifacts are evidence, not source Retained live/visual evidence belongs under `evals/results//...` and CI artifacts. Do not treat generated result JSON, screenshots, event streams, or temporary projects as canonical repository state unless a fixture intentionally owns them.