Skip to content

ci: auto-sort and validate systems.csv on PRs - #894

Open
fredericsimard wants to merge 8 commits into
masterfrom
chore/sort-validate-systems-csv/2026-07-24
Open

ci: auto-sort and validate systems.csv on PRs#894
fredericsimard wants to merge 8 commits into
masterfrom
chore/sort-validate-systems-csv/2026-07-24

Conversation

@fredericsimard

Copy link
Copy Markdown
Contributor

What

Adds automated checks that run on every pull request that changes systems.csv, plus a data sort so the catalog starts in a sorted state.

New workflow: .github/workflows/systems_csv_pr_checks.yml

Job Behavior
sort Re-sorts systems.csv and pushes the result back to the PR branch as a separate chore: sort systems.csv commit. Same-repo branches only — fork PRs are skipped (the default GITHUB_TOKEN cannot push to forks).
validate-structure Fails on malformed rows (wrong field count / broken quoting) and empty required columns (Country Code, Name, Location, System ID, URL, Auto-Discovery URL). Empty Supported Versions → warning; the 3 Authentication columns are optional. Emits inline PR annotations.
check-new-urls For URLs newly added by the PR (both URL and Auto-Discovery URL columns), follows redirects and requires a final HTTP 200–299. Pre-existing URLs are ignored.

New scripts (scripts/)

  • sort-systems-csv.sh — mirrors the canonical (head -n 1; LC_ALL=en_US.UTF-8 sort -t, -k1,1 -k2,2) < systems.csv. Supports --check and a GNUSORT=gsort override for macOS.
  • validate-systems-csv.js — structure validator (Node, no deps).
  • check-new-urls.js — new-URL HTTP checker (Node, no deps; retries once on transient network errors).

Other changes

  • chore: sort systems.csv — applies the sort now (3 rows reordered).
  • README.md — the "keep this list alphabetized" note now explains the automated sort in plain and technical terms, with local-repro instructions.

Notes / decisions

  • Fork PRs: auto-sort commit is skipped (can't push to forks); structure and URL checks still run.
  • URL check covers both URL columns per request; operator homepages occasionally return 403/429 to CI bots, which would fail a PR — worth watching in practice.

Test evidence (local)

  • Sort: idempotent; --check correctly flags unsorted input.
  • Structure: current file passes with 1 warning (empty Supported Versions); synthetic malformed rows correctly flagged.
  • URL check: pre-existing URLs ignored, new URLs from both columns checked, passes on 2xx / fails on non-2xx.

🤖 Generated with Claude Code

fredericsimard and others added 3 commits July 24, 2026 14:56
Adds a GitHub Actions workflow that runs on any pull request changing
systems.csv, plus the scripts it drives:

- sort: re-sorts systems.csv (header kept in place, remaining rows sorted
  by Country Code then Name with GNU sort under en_US.UTF-8) and pushes the
  result back to the PR branch as a separate `chore: sort systems.csv`
  commit. Same-repo branches only; fork PRs are skipped because the default
  GITHUB_TOKEN cannot push to forks.
- validate-structure: fails on malformed rows (wrong field count / broken
  quoting) and on empty required columns (Country Code, Name, Location,
  System ID, URL, Auto-Discovery URL). An empty Supported Versions is a
  non-fatal warning; the three Authentication columns are optional.
- check-new-urls: for URLs newly added by the PR (both the URL and
  Auto-Discovery URL columns), follows redirects and requires a final
  HTTP 200-299.

scripts/sort-systems-csv.sh mirrors the canonical one-liner and supports a
--check mode plus a GNUSORT override for macOS (gsort). README documents the
now-automated sorting in plain and technical terms.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Apply the canonical sort so the catalog starts in a sorted state; going
forward the systems.csv PR checks workflow keeps it sorted automatically.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you all sign our Contributor License Agreement before we can accept your contribution.
1 out of 2 committers have signed the CLA.

✅ fredericsimard
❌ github-actions[bot]
You have signed the CLA already but the status is still pending? Let us recheck it.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed by subsequent commits

Pull request overview

Adds automated pull-request checks around systems.csv to keep the catalog consistently sorted and to validate new/changed entries, with supporting scripts and contributor documentation updates.

Changes:

  • Introduces a new PR workflow to auto-sort systems.csv, validate CSV structure/required fields, and HTTP-check newly added URLs.
  • Adds three new helper scripts under scripts/ to implement sorting, structural validation, and new-URL checking.
  • Applies an initial sort to systems.csv and updates README contributor guidance with local reproduction instructions.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 7 comments.

Show a summary per file
File Description
systems.csv Reorders a handful of rows to match the new canonical sort order.
scripts/validate-systems-csv.js Adds a Node-based structure/required-field validator emitting GitHub Actions annotations.
scripts/sort-systems-csv.sh Adds an in-repo canonical sort implementation (header preserved; GNU sort keys).
scripts/check-new-urls.js Adds a Node-based checker that verifies newly introduced URLs return final 2xx after redirects.
README.md Updates contributor instructions to describe the automated sort and local reproduction steps.
.github/workflows/systems_csv_pr_checks.yml New workflow wiring sorting + validation + URL checks on PRs touching systems.csv.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread scripts/validate-systems-csv.js
Comment thread scripts/validate-systems-csv.js Outdated
Comment thread .github/workflows/systems_csv_pr_checks.yml
Comment thread README.md Outdated
Comment thread scripts/check-new-urls.js Outdated
Comment thread scripts/check-new-urls.js
Comment thread scripts/check-new-urls.js
The CI auto-sort (Linux/glibc) produced a different order than a local
macOS sort under the same en_US.UTF-8 locale, so the two would fight in a
loop. Switch the canonical sort to LC_ALL=C (byte-wise), which is identical
on macOS BSD sort and Linux GNU sort; re-sort systems.csv accordingly and
update the README to describe the byte-wise ordering and drop the macOS
coreutils requirement.

Also addresses adversarial PR review on the systems.csv checks:

- check-new-urls.js: add an SSRF guard. URLs come from untrusted fork PRs
  and are fetched by the runner, so refuse to connect to loopback, private,
  link-local, CGNAT, and ULA addresses (and localhost), resolving hostnames
  and re-validating every redirect hop. Blocks probing of the runner network
  and cloud metadata endpoints (e.g. 169.254.169.254).
- check-new-urls.js: cap newly added URLs checked per PR (MAX_NEW_URLS=200).
- validate-systems-csv.js: escape %/CR/LF in GitHub Actions annotations so
  multi-line errors (e.g. header mismatch) are not truncated; fix the
  blank-line comment to match behavior.
- workflow: gate check-new-urls on validate-structure.
- README: fix GitHub branding ("Github-less" -> "GitHub-less").

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 5 out of 6 changed files in this pull request and generated 2 comments.

Comments suppressed due to low confidence (1)

scripts/check-new-urls.js:163

  • The SSRF guard can be bypassed via DNS rebinding: hostIsSafe() resolves the hostname and checks the returned IPs, but http(s).request() will do its own DNS lookup at connection time (which may return a different/private address). Add a lookup option to the request that validates the resolved address with isBlockedIp() before connecting, so the safety check is enforced at the actual socket connection step as well.
      {
        method: 'GET',
        headers: { 'User-Agent': USER_AGENT, Accept: '*/*' },
        timeout: TIMEOUT_MS,
      },

Comment thread scripts/sort-systems-csv.sh
Comment thread .github/workflows/systems_csv_pr_checks.yml Outdated
The previous sort used `sort -t, -k1,1 -k2,2`, which is not aware of CSV
quoting: a Name quoted because it contains a comma was sorted by the text up
to the first raw comma, and (under LC_ALL=C) its leading `"` byte pushed the
row to the top of its country block. Two rows were affected -- "Veo
University of Illinois, Urbana-Champaign" (US) and "Styr & Ställ (Sweden,
Göteborg)" (SE).

Replace the bash sorter with a small CSV-quoting-aware Node sorter
(scripts/sort-systems-csv.js) that parses quoted fields, sorts by the real
Country Code then Name using byte-wise (LC_ALL=C-equivalent) comparison, and
re-emits each row verbatim. Implementing the comparison in Node keeps the
result identical on macOS and the Linux CI runner. Re-sort systems.csv so the
two quoted rows move to their correct alphabetical positions; update the
workflow to run the Node sorter and the README to match.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@richfab

richfab commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Thank you @fredericsimard.

Is it possible to do the sorting on the first four columns (as mentioned here) please?

Indeed, some operators like "Check Technologies" and "Bolt Technology OÜ" use the same name in all their locations.

Extend the sort key from (Country Code, Name) to the first four columns, in
order: Country Code, Name, Location, System ID. This makes the ordering of
rows that share a Country Code + Name (e.g. the 23 "Check Technologies" rows
in NL, or "Bolt Technology OÜ" across several German cities) explicit and
deterministic by Location then System ID.

systems.csv itself is unchanged: the previous full-row tie-break already
ordered such rows by the bytes following the Name (which begin with Location
and System ID), so the current data is already in the new order.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@fredericsimard

Copy link
Copy Markdown
Contributor Author

@richfab The sort has been updated to use the first 4 columns, the sort was applied and produced no changes. README has been updated to reflect the 4-column key sorting.

Provide scripts/sort-systems-csv.sh as a shell alternative to
scripts/sort-systems-csv.js, for contributors who would rather not depend on
Node.js. It is manual-only; the CI workflow continues to run the Node sorter.

Both implementations are CSV-quoting-aware, sort by the first four columns
(Country Code, Name, Location, System ID) byte-wise, and produce byte-identical
output. The shell version decorates each row with its four sort keys separated
by a control character that cannot occur in the CSV, sorts under LC_ALL=C, then
strips the key prefix with cut.

Verified equivalent on: a shuffled copy of the full systems.csv (both restore
the committed order exactly), quoted fields containing commas, blank lines, and
malformed input.

Also converge two behaviors that had diverged between the two scripts: blank
data lines are now dropped by both (rather than the Node one sorting them to
the top), and malformed CSV exits 3 from both, with the Node sorter reporting a
clean error message instead of an uncaught stack trace.

README documents both commands and the --check flag.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

@davidgamez davidgamez left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@fredericsimard

Copy link
Copy Markdown
Contributor Author

Cross-platform verification of the sort on Ubuntu/Linux

Since the auto-sort job commits back to the PR branch, the sort has to be bit-for-bit reproducible across platforms — if a contributor's local sort disagrees with the runner's, the two fight in a loop and the bot pushes a re-sort on every push. This PR already hit that failure mode once: the original implementation used LC_ALL=en_US.UTF-8, and the Linux runner reordered 25 rows relative to a local macOS sort, producing an unwanted chore: sort systems.csv commit (88ef2a1). The fix was to switch to byte-wise ordering and to implement the comparison in-script rather than delegating collation to the platform.

This comment records the verification that the fix actually holds on Linux.

Test environment

Run under Ubuntu in a container, i.e. the same family as ubuntu-latest on the runner:

Component Version
Distro Ubuntu 26.04 LTS
awk mawk 1.3.4 20260129
sort uutils coreutils 0.8.0

Two things here are deliberately unlike the macOS dev environment, which is what makes the test meaningful:

  • awk is mawk, not the BSD/one-true-awk (awk version 20200816) used on macOS. The shell sorter parses CSV fields in awk, so a behavioral difference here would show up directly in the ordering.
  • sort is uutils coreutils (the Rust reimplementation) rather than GNU coreutils. The shell sorter's ordering therefore does not depend on any GNU-specific sort behavior.

What was checked

  1. --check against the committed systems.csv — reports already sorted, exit 0.
  2. Shuffled round-trip: all 1520 data rows shuffled, then re-sorted on Linux with scripts/sort-systems-csv.sh, and compared to the committed file. This is the real test — a no-op --check on an already-sorted file proves very little on its own.
  3. Placement of the two CSV-quoted rows whose Name contains a comma, which were the subject of the CSV-quoting review comment.

Results

=== 1. --check on pristine committed file ===
/tmp/orig.csv is already sorted.
exit=0

=== 2. shuffle all rows, re-sort, compare to committed order ===
Sorted /tmp/shuf.csv.
PASS: Linux sorter reproduces the committed order byte-for-byte

=== 3. quoted-comma rows placed correctly? ===
1516:US,"Veo University of Illinois, Urbana-Champaign","Urbana, IL",veo-iuc,...
1311:SE,"Styr & Ställ (Sweden, Göteborg)",Göteborg,nextbike_zg,...

Both quoted rows land at the same line numbers as on macOS (1516 and 1311), in their correct alphabetical positions rather than pinned to the top of their country block.

Conclusion

Three independent implementations agree byte-for-byte on the full dataset:

Implementation Platform
scripts/sort-systems-csv.js (Node) macOS
scripts/sort-systems-csv.sh (bash + BSD awk) macOS
scripts/sort-systems-csv.sh (bash + mawk + uutils sort) Ubuntu 26.04

This is consistent with what CI now shows: the Sort systems.csv job logs systems.csv is already sorted; no changes. and pushes no commit, whereas the earlier en_US.UTF-8 version did.

Reproducing

Any Linux environment works. Using Apple's container on macOS:

container run --rm -v "$PWD:/repo" -w /tmp ubuntu:latest bash -lc '
  cp /repo/systems.csv /tmp/o.csv
  cp /repo/scripts/sort-systems-csv.sh /tmp/
  { head -n1 /tmp/o.csv; tail -n +2 /tmp/o.csv | shuf; } > /tmp/s.csv
  bash /tmp/sort-systems-csv.sh /tmp/s.csv >/dev/null
  cmp -s /tmp/s.csv /tmp/o.csv && echo PASS || { echo FAIL; diff /tmp/s.csv /tmp/o.csv | head; }'

The script copies to /tmp before sorting, so the mounted working tree is never modified. The equivalent Docker invocation is the same command with docker in place of container.

Comment thread systems.csv Outdated
Comment on lines +39 to +41
AT,city bike Linz,Linz,nextbike_al,https://citybikelinz.at/,https://gbfs.nextbike.net/maps/gbfs/v2/nextbike_al/gbfs.json,2.3,,,
AT,nextbike Klagenfurt Austria,Klagenfurt,nextbike_ka,https://www.nextbike.at/de/klagenfurt/,https://gbfs.nextbike.net/maps/gbfs/v2/nextbike_ka/gbfs.json,2.3,,,
AT,nextbike Niederösterreich,Austria,nextbike_la,https://www.nextbike.at/niederoesterreich/,https://gbfs.nextbike.net/maps/gbfs/v2/nextbike_la/gbfs.json,2.3,,,

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fredericsimard Is possible to ignore the case? "nextbike" should appear after "LiBike". Thank you in advance

@richfab

richfab commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

@fredericsimard I'd like to take some time to rewrite the section about the sorting in the README, which I will do when I return from vacation on August 31, 2026.

Indeed, external contributions will always come from a fork (as far as I know) and I can see in the README that the automated sorting action cannot run for PRs in forks.

Let's put this on pause until I return. Thank you very much for your support on this.

Byte-wise ordering put every uppercase-initial name ahead of every
lowercase-initial one, which read as wrong within a country block: in AT,
"city bike Linz" was stranded below "WienMobil Rad", and "nextbike ..." sat at
the bottom instead of directly after "LiBike".

Fold A-Z to a-z when building the four sort keys so ordering ignores case.
Rows whose keys differ only in case fall back to a byte-wise comparison of the
whole row, so the result stays deterministic.

Case folding is deliberately ASCII-only rather than full Unicode: JavaScript's
toLowerCase() and awk's tolower() disagree about non-ASCII characters, so using
each language's natural function would make the Node and shell sorters produce
different orderings. Accented names are therefore still compared by their UTF-8
bytes and sort after ASCII names; this is documented in the README. The awk
pipeline also now runs under LC_ALL=C so the fold cannot be perturbed by the
ambient locale.

Re-sorted systems.csv accordingly (93 rows moved).

Verified: the Node and shell sorters produce byte-identical output on the full
file, on rows differing only by case, and on accented names; and a shuffled
copy of systems.csv re-sorted under Ubuntu 26.04 / mawk 1.3.4 reproduces the
committed order byte-for-byte.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@fredericsimard

Copy link
Copy Markdown
Contributor Author

@richfab no problem. I fixed the sorting issue, it's been committed to this PR. See 6a98ab2 (this PR)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants