Run BabelTest assertions embedded in GitHub issues as pytest tests

.github/workflows/tests.yaml

@@ -0,0 +1,37 @@
+name: Tests
+
+on:
+  pull_request:
+
+# Only checkout needs the token. Declaring this at all matters: every scope not
+# listed here is set to none, so the job no longer inherits whatever the org's
+# default-permissions setting happens to be. The unit tests themselves need no
+# GitHub API access — `-m unit` deselects the issue tests before they look for a
+# token — so anything beyond contents:read would be unused.
+permissions:
+  contents: read
+
+jobs:
+  unit-tests:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+
+      - uses: astral-sh/setup-uv@v6
+
+      # To run the GitHub issue tests here too — drop the `-m unit` filter, or
+      # add a second job running `uv run pytest tests/github_issues` — put back:
+      #
+      #     env:
+      #       GITHUB_TOKEN: ${{ github.token }}
+      #
+      # and add `issues: read` to the permissions block above. Do not skip the
+      # env: block — without a token the issue tests skip at the module level
+      # rather than failing, so CI would go green having tested nothing.
+      #
+      # The token covers this repository only: the other repos in targets.ini's
+      # Repositories list are readable because they are public, not because the
+      # token is scoped to them, so adding a private one would mean a PAT in a
+      # secret instead.
+      - name: Run unit tests
+        run: uv run pytest -m unit -v

.gitignore

@@ -1,3 +1,6 @@
+# Ignore the root .env file.
+/.env
+
 # Ignore all data files.
 data/
 

CLAUDE.md

@@ -29,6 +29,10 @@ pytest tests/nodenorm/test_nodenorm_from_gsheet.py -k "row=42"  # Run a specific
 black tests/    # Format Python test code
 ```
 
+Note that the repository is *not* currently black-clean — `black --check tests/ src/` reports
+~30 files it would reformat. Running `black` across the tree would bury a real change in
+unrelated churn, so format only the files you touch, or match the surrounding style.
+
 ### Vue Website (website-vue3-vite/)
 
 ```bash
@@ -76,6 +80,56 @@ The core of this project. Tests validate NodeNorm and NameRes services across mu
 - **`website/`** — Newer Astro-based site deployed to GitHub Pages with prefix comparator and autocomplete tools
 - **`scala-validation/`** — Legacy, unmaintained
 
+## Untrusted Input
+
+Most of what this project reads was written by someone else and reviewed by nobody. Treat it as
+hostile, not merely as data that might be malformed:
+
+- **GitHub issue bodies** (`src/babel_validation/sources/github/`) — anyone with a GitHub account
+  can write one, and we parse it into live NodeNorm/NameRes calls.
+- **The Google Sheet** (`src/babel_validation/sources/google_sheets/`) — anyone with edit access.
+- **Anything off the network**, including a service's response.
+
+`tests/targets.ini` is the exception: its URLs and its `Repositories` list are checked-in config,
+so they are trusted, and guards belong on what the issue supplies rather than on them.
+
+Every failure mode below was real, and found in this code. These are the shapes to look for.
+
+**A regex over untrusted text can hang the process.** `\s+ .*? \s+` before a literal is three
+nested backtracking quantifiers, and matched in cubic time: 53s on an 8KB body, hours at GitHub's
+65536-character limit. Avoid adjacent quantifiers that can match the same characters — anchor on
+something disjoint, such as a newline or a literal. Note that `pytest --timeout` only wraps test
+execution, so anything running at **collection** time has no timeout at all.
+
+**`yaml.safe_load` is not a safe parser, only a non-executing one.** It still resolves anchors,
+aliases and merge keys, and PyYAML shares the aliased nodes rather than copying them — so the load
+looks cheap and the blow-up lands on whatever formats the result afterwards. 337 bytes became a
+25MB error message. Use `_NoAliasSafeLoader` in `sources/github/github_issues_test_cases.py`.
+
+**Format untrusted text with `%r` / `!r`, never `%s` / `{}`.** `repr()` escapes exactly the
+characters `str.isprintable()` rejects — ANSI escapes, C0/C1 controls, bidi overrides, zero-width
+characters — so it is the whole defence for anything reaching a terminal, a log line or a pytest
+ID. Truncate before `repr()`ing anything that might be large: the message is kept in pytest's
+report.
+
+**A guard that runs after the value was logged is too late.** Validate at the one choke point that
+sees every value before anything formats it. For assertion params that is
+`AssertionHandler._rejection()`, because the per-handler CURIE check skips whatever
+`curie_params()` excludes and is turned off entirely by `VALIDATE_CURIES = False`.
+
+**Never let outside text choose what we fetch.** `get_issues_by_ids()` takes an ID that decides
+which repository we read assertions from, and its `[^#]+` group admits slashes — so check the
+allowlist *before* the call, or the value reaches the GitHub API as a URL path.
+
+**Fail loudly; skipping looks like passing.** Reject a bad issue rather than silently running a
+truncated part of it. The same goes for missing credentials: the GitHub issue tests *skip* without
+a token, so a green run may have tested nothing.
+
+**Caches belong in `cache_dir()`** (`src/babel_validation/core/__init__.py`), a 0700 directory
+under the user's home — never a fixed name in the shared temp directory. On a CI runner or a
+shared machine anyone can pre-create such a file, and the issue cache decides what a later run
+fetches and executes.
+
 ## Key Dependencies
 
 - Python >=3.11, pytest, requests, deepdiff, openapi-spec-validator, black
@@ -88,4 +142,27 @@ When writing new tests:
 - For Google Sheet-based tests, parametrize with `gsheet.test_rows()` and use the `test_category` fixture for category filtering
 - Use `pytest.mark.xfail(strict=True)` for known failures (strict=True means unexpected passes also fail)
 - Hand-written per-issue regression tests go in `tests/nodenorm/by_issue/`
+- **`pytest tests/github_issues` is expected to be red, and that is the tool working.** An open
+  issue whose assertions all pass is a strict XPASS, meaning it looks closeable; a closed issue
+  with failing assertions means it looks like it should be reopened. Those results are findings
+  about Babel, not defects in this repo — do not "fix" them by editing the assertions. Only a
+  hard ERROR (an unknown assertion name, a rejected issue body) is a problem here.
+- When checking that a new test really fails without its fix, **clear `__pycache__` between runs**.
+  A same-length edit (`%r` for `%s`, say) leaves the source's size unchanged, and if the mtime lands
+  in the same granularity the `.pyc` is not invalidated — so the mutation appears to pass a test
+  that never saw it. Also avoid asserting on `caplog.text` for anything about control characters:
+  it does not carry them through, so such a test passes whatever the code does. Read
+  `caplog.records` and `getMessage()` instead.
+- **Never put a complete `{{BabelTest|...}}` marker or a fenced `babel_tests:` block into a GitHub
+  issue you file** — not even in prose explaining the syntax. `TranslatorSRI/babel-validation` is
+  itself in the scanned `Repositories` list, so the harness collects the marker and runs it: an
+  issue that merely *describes* an assertion becomes a test of that assertion. Because a new issue
+  is open, an assertion that passes then reports as a strict XPASS failure. This is not
+  hypothetical — issue #115 was filed with a marker in it and immediately failed the live suite.
+  Quote a partial marker instead, dropping the closing `}}`, which the pattern needs to match. A
+  one-line ```` ```yaml babel_tests: ``` ```` in prose is already safe: the block pattern requires a
+  newline after the key.
+- To check behaviour when no GitHub token is available, run with `GITHUB_TOKEN=` (set but
+  empty) rather than unsetting it: `dotenv.load_dotenv()` will not override a key already
+  present in `os.environ`, so this defeats the token in the developer's `.env` file
 - Import shared classes from `src.babel_validation.*` (e.g. `from src.babel_validation.services.nodenorm import CachedNodeNorm`)

README.md

@@ -61,6 +61,66 @@ ssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssssss
 ======================================================= 41 passed, 1965 skipped, 4 xfailed in 10.11s ========================================================
 ```
 
+### GitHub issue tests
+
+Assertions can also be embedded directly in GitHub issue bodies — see
+[`src/babel_validation/assertions/README.md`](./src/babel_validation/assertions/README.md)
+for the syntax and the available assertion types. The repositories scanned for them are
+listed under `Repositories` in the `[DEFAULT]` section of
+[`tests/targets.ini`](./tests/targets.ini).
+
+Issue bodies are untrusted input, so the harness caps what one issue may contain — 100
+assertions, 1,000 params lists, 1,000 parameters, 1,000 characters per parameter — and
+rejects YAML anchors, aliases and duplicate keys. An issue over a cap fails loudly rather
+than running part of itself; split it into several issues. The caps are listed in
+[`src/babel_validation/assertions/README.md`](./src/babel_validation/assertions/README.md).
+`--issue` resolves only within the configured `Repositories`, so a run can never be pointed
+at assertions from somewhere else.
+
+Beware when *discussing* the syntax in an issue: a complete `{{BabelTest|...}}` marker is
+picked up wherever it appears, backticks included, and an unrecognised assertion name fails
+the run rather than being ignored. Quote a partial marker instead — the pattern needs the
+closing `}}` to match.
+
+```shell
+$ pytest tests/github_issues --target dev                       # every issue carrying assertions
+$ pytest tests/github_issues --target dev --issue 'org/repo#42' # just one (also 'repo#42' or '42')
+```
+
+These tests need a `GITHUB_TOKEN`, in the environment or in a `.env` file. Without one they
+**skip rather than fail**, so a run can look green having tested nothing. Generate a
+[personal access token](https://docs.github.com/en/authentication/keeping-your-account-and-data-secure/managing-your-personal-access-tokens);
+inside a GitHub Action, use the
+[automatic `GITHUB_TOKEN`](https://docs.github.com/en/actions/security-for-github-actions/security-guides/automatic-token-authentication)
+instead.
+
+The token is not needed for authentication as such — every repository we scan is public, and
+both the single-issue and search endpoints answer unauthenticated requests. It is needed for
+the [rate limits](https://docs.github.com/en/rest/using-the-rest-api/rate-limits-for-the-rest-api):
+
+| | Unauthenticated | With a token |
+| --- | --- | --- |
+| Core | 60 / hour, **per IP** | 5,000 / hour |
+| [Search](https://docs.github.com/en/rest/search/search) | 10 / minute | 30 / minute |
+
+Discovery is search-bound, not core-bound: two searches per configured repository (one per
+trigger keyword, plus a request per extra page of results), and then no core request at all,
+because a search result already carries the issue `body` and `html_url` the harness needs.
+Scanning the five configured repositories currently finds 96 issues for zero core requests.
+
+Core requests are spent re-hydrating issues one at a time, which happens whenever the cached
+ID list is reused instead of the search being repeated — notably in every `pytest-xdist`
+worker after the first. That path costs one request per issue per worker, so an
+unauthenticated run would exhaust the 60/hour core budget well before finishing.
+
+`GET /rate_limit` reports what is left without itself counting against the limit
+([docs](https://docs.github.com/en/rest/rate-limit/rate-limit)). Note that the search window
+resets every 60 seconds, so its counter is often back at zero by the time you look:
+
+```shell
+$ curl -s -H "Authorization: Bearer $GITHUB_TOKEN" https://api.github.com/rate_limit
+```
+
 ## Log Analysis
 
 The Jupyter Notebook in `log-analysis/` contains some basic analysis of the

pyproject.toml

@@ -7,12 +7,17 @@ readme = "README.md"
 requires-python = ">=3.11"
 dependencies = [
     "black>=25.9.0",
+    "pyyaml>=6.0",
     "requests>=2.32.5",
     "filelock",
     "deepdiff>=8.6.1",
+    "python-dotenv>=0.9.9",
     "openapi-spec-validator>=0.7.2",
-    "pytest>=8.4.2",
+    "pygithub>=2.8.1",
+    "pytest>=9.0.2",
     "pytest-timeout>=2.4.0",
+    "pytest-xdist[psutil]",
+    "pytest-subtests",
 ]
 
 [project.urls]

src/babel_validation/assertions/README.md

@@ -39,6 +39,29 @@ The meaning of each element in a params list depends on the assertion type (see
 For most assertions the elements are CURIEs; for `HasLabel` the second element is a
 label string; for `ResolvesWithType` the first element is a Biolink type.
 
+## Limits
+
+Issue bodies are untrusted input — anyone can write one, and nothing reviews it before the
+harness parses it and turns it into live NodeNorm and NameRes calls. These caps bound what
+one issue can cost. They sit far above anything a real issue contains:
+
+| Limit | Value |
+| --- | --- |
+| Assertions per issue | 100 |
+| Params lists per issue | 1,000 |
+| Parameters per issue | 1,000 |
+| Characters per parameter | 1,000 |
+
+Exceeding one of the first three fails the whole issue rather than running part of it — split
+the assertions across several issues. An individual parameter that is too long, empty, or
+contains non-printable characters fails only its own params list; the rest of the issue still
+runs.
+
+YAML anchors and aliases (`&name` / `*name`, including merge keys) are rejected: they let a few
+hundred bytes expand into megabytes. Duplicate keys in a `babel_tests` block are rejected too,
+since YAML would silently keep only the last one — and then the block a reviewer reads would
+not be the one that runs.
+
 ---
 
 ## NodeNorm Assertions
@@ -248,15 +271,21 @@ babel_tests:
    These are rendered into this file, so write them for someone reading this README
    rather than for someone reading the class.
 
-3. Implement `test_params_list()` (or both `test_with_*` methods for `AssertionHandler`
-   subclasses). It receives one params_list at a time, already stripped and — unless the
-   handler sets `VALIDATE_CURIES = False` — with its CURIEs validated and pre-warmed in
-   the NodeNorm cache. Yield one result per thing checked, usually one per CURIE, so a
-   failure names the CURIE that failed. Override `curie_params()` if some params are not
-   CURIEs; see `HasLabel` and `SearchByName`.
+3. Declare how many params a params_list may have with `MIN_PARAMS` and `MAX_PARAMS`
+   (default: one or more). Arity is checked during preparation, so a params_list of the
+   wrong length is rejected before any CURIE is looked up and `test_params_list()` never
+   sees it — do not re-check it by hand.
+
+4. Implement `test_params_list()` (or both `test_with_*` methods for `AssertionHandler`
+   subclasses). It receives one params_list at a time, of a length you declared, already
+   stripped and — unless the handler sets `VALIDATE_CURIES = False` — with its CURIEs
+   validated and pre-warmed in the NodeNorm cache, so you can index into it directly.
+   Yield one result per thing checked, usually one per CURIE, so a failure names the CURIE
+   that failed. Override `curie_params()` if some params are not CURIEs; see `HasLabel`
+   and `SearchByName`.
 
-4. Import it in `__init__.py` and add an instance to `ASSERTION_HANDLERS`. Order does not
+5. Import it in `__init__.py` and add an instance to `ASSERTION_HANDLERS`. Order does not
    matter — this file groups handlers by the service they test.
 
-5. Run `uv run python -m src.babel_validation.assertions.gen_docs` to regenerate `README.md`,
+6. Run `uv run python -m src.babel_validation.assertions.gen_docs` to regenerate `README.md`,
    and `uv run pytest -m unit` to confirm the checked-in copy is in sync.