From e251f177b578cc1158f3464906ffe588c40b7836 Mon Sep 17 00:00:00 2001 From: Gaurav Vaidya Date: Fri, 26 Jun 2026 00:52:19 -0400 Subject: [PATCH 01/23] Add assertions framework for BabelTest expectations Introduce src/babel_validation/assertions: a small framework of handler classes that turn a named BabelTest assertion (HasLabel, SearchByName, ResolvesWith, DoesNotResolveWith, ResolvesWithType, Needed, ...) plus its parameters into a check evaluated against NodeNorm/NameRes, returning a TestResult. assertions/README.md is generated from the handler class attributes by gen_docs.py, and test_assertions_docs.py (marked `unit`, offline) asserts the checked-in README stays in sync. Register the `unit` marker, first used by that test. Co-Authored-By: Claude Opus 4.8 --- pyproject.toml | 3 + src/babel_validation/assertions/README.md | 243 +++++++++++++++++ src/babel_validation/assertions/__init__.py | 187 +++++++++++++ src/babel_validation/assertions/common.py | 13 + src/babel_validation/assertions/gen_docs.py | 146 ++++++++++ src/babel_validation/assertions/nameres.py | 61 +++++ src/babel_validation/assertions/nodenorm.py | 249 ++++++++++++++++++ .../test_environment/test_assertions_docs.py | 14 + 8 files changed, 916 insertions(+) create mode 100644 src/babel_validation/assertions/README.md create mode 100644 src/babel_validation/assertions/__init__.py create mode 100644 src/babel_validation/assertions/common.py create mode 100644 src/babel_validation/assertions/gen_docs.py create mode 100644 src/babel_validation/assertions/nameres.py create mode 100644 src/babel_validation/assertions/nodenorm.py create mode 100644 tests/test_environment/test_assertions_docs.py diff --git a/pyproject.toml b/pyproject.toml index c5fbd4ea..80f4ba02 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -32,3 +32,6 @@ packages = ["src"] # (including node_modules) during collection. testpaths = ["tests"] timeout = 300 +markers = [ + "unit: unit tests that do not require network access", +] diff --git a/src/babel_validation/assertions/README.md b/src/babel_validation/assertions/README.md new file mode 100644 index 00000000..9cc4ef7d --- /dev/null +++ b/src/babel_validation/assertions/README.md @@ -0,0 +1,243 @@ + + +# BabelTest Assertion Types + +This package defines the assertion types that can be embedded in GitHub issue bodies and evaluated against the NodeNorm and NameRes services. + +## Embedding Tests in Issues + +Two syntaxes are supported: + +**Wiki syntax** (one assertion per line): +``` +{{BabelTest|AssertionType|param1|param2|...}} +``` + +**YAML syntax** (multiple assertions, multiple param sets): +```` +```yaml +babel_tests: + AssertionType: + - param1 + - [param1, param2] +``` +```` + +Assertion names are case-insensitive, as is the `{{BabelTest|...}}` marker itself. + +## Param Sets + +Each assertion can be invoked with one or more **param sets** — independent groups of +parameters that are each evaluated separately. + +- **Wiki syntax** — each `{{BabelTest|...}}` line is one param set. +- **YAML syntax** — each list entry under an assertion key is one param set; a bare string + is a single-element param set, a YAML list is a multi-element param set. + +The meaning of each element in a param set depends on the assertion type (see below). +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. + +--- + +## NodeNorm Assertions + +These assertions test the [NodeNorm](https://nodenorm.transltr.io/docs) service. + +### Resolves + +**Applies to:** NodeNorm + +Each CURIE in each param_set must resolve to a non-null result in NodeNorm. + +**Parameters:** One or more CURIEs per param_set. + +**Wiki syntax:** +``` +{{BabelTest|Resolves|CHEBI:15365}} +{{BabelTest|Resolves|MONDO:0005015|DOID:9351}} +``` + +**YAML syntax:** +```yaml +babel_tests: + Resolves: + - CHEBI:15365 + - [MONDO:0005015, DOID:9351] +``` + +--- + +### DoesNotResolve + +**Applies to:** NodeNorm + +Each CURIE in each param_set must fail to resolve (return null) in NodeNorm. Use this to confirm that an identifier is intentionally not normalizable. + +**Parameters:** One or more CURIEs per param_set. + +**Wiki syntax:** +``` +{{BabelTest|DoesNotResolve|FAKENS:99999}} +``` + +**YAML syntax:** +```yaml +babel_tests: + DoesNotResolve: + - FAKENS:99999 +``` + +--- + +### ResolvesWith + +**Applies to:** NodeNorm + +All CURIEs within each param_set must resolve to the identical normalized result. Use this to assert that two identifiers are equivalent. + +**Parameters:** Two or more CURIEs per param_set. All must resolve to the same result. + +**Wiki syntax:** +``` +{{BabelTest|ResolvesWith|CHEBI:15365|PUBCHEM.COMPOUND:1}} +``` + +**YAML syntax:** +```yaml +babel_tests: + ResolvesWith: + - [CHEBI:15365, PUBCHEM.COMPOUND:1] + - [MONDO:0005015, DOID:9351] +``` + +--- + +### DoesNotResolveWith + +**Applies to:** NodeNorm + +The CURIEs within each param_set must NOT all resolve to the same normalized result. Use this to assert that two identifiers are intentionally distinct entities. + +**Parameters:** Two or more CURIEs per param_set. They must not all resolve to the same result. + +**Wiki syntax:** +``` +{{BabelTest|DoesNotResolveWith|CHEBI:15365|CHEBI:16856}} +``` + +**YAML syntax:** +```yaml +babel_tests: + DoesNotResolveWith: + - [CHEBI:15365, CHEBI:16856] +``` + +--- + +### HasLabel + +**Applies to:** NodeNorm + +The CURIE must resolve in NodeNorm and its primary label (id.label) must match the expected label exactly (case-sensitive). + +**Parameters:** Exactly two elements per param_set: a CURIE, then the expected label string. + +**Wiki syntax:** +``` +{{BabelTest|HasLabel|CHEBI:15365|aspirin}} +``` + +**YAML syntax:** +```yaml +babel_tests: + HasLabel: + - [CHEBI:15365, aspirin] +``` + +--- + +### ResolvesWithType + +**Applies to:** NodeNorm + +Each param_set must have at least two elements: the first is the expected Biolink type (e.g. 'biolink:Gene'), and the remainder are CURIEs that must resolve with that type. + +**Parameters:** Each param_set: first element is the expected Biolink type (e.g. `biolink:Gene`), remaining elements are CURIEs. + +**Wiki syntax:** +``` +{{BabelTest|ResolvesWithType|biolink:Gene|NCBIGene:1}} +``` + +**YAML syntax:** +```yaml +babel_tests: + ResolvesWithType: + - [biolink:Gene, NCBIGene:1, HGNC:5] +``` + +--- + +## NameRes Assertions + +These assertions test the [NameRes](https://name-lookup.transltr.io/docs) service. + +### SearchByName + +**Applies to:** NameRes + +Each param_set must have at least two elements: a search query string and an expected CURIE. The test passes if the CURIE's normalized identifier appears within the top N results (default N=5) when NameRes looks up the search query. + +**Parameters:** Each param_set: the **search query string** and the **expected CURIE**. The CURIE is normalized via NodeNorm (drug/chemical conflation enabled) before matching. + +**Wiki syntax:** +``` +{{BabelTest|SearchByName|water|CHEBI:15377}} +``` + +**YAML syntax:** +```yaml +babel_tests: + SearchByName: + - [water, CHEBI:15377] + - [diabetes, MONDO:0005015] +``` + +--- + +## Special Assertions + +### Needed + +**Applies to:** NodeNorm and NameRes + +Marks an issue as needing a test — always fails as a reminder to add real assertions. + +**Wiki syntax:** +``` +{{BabelTest|Needed}} +``` + +**YAML syntax:** +```yaml +babel_tests: + Needed: + - placeholder +``` + +--- + +## Adding a New Assertion Type + +1. Choose the right module: + - `nodenorm.py` — for NodeNorm-only assertions (subclass `NodeNormTest`, override `test_param_set`) + - `nameres.py` — for NameRes-only assertions (subclass `NameResTest`, override `test_param_set`) + - `common.py` — for assertions that apply to both services (subclass `AssertionHandler`, override `test_with_nodenorm` and/or `test_with_nameres`) + +2. Define the class with `NAME`, `DESCRIPTION`, `PARAMETERS`, `WIKI_EXAMPLES`, `YAML_PARAMS`, and `test_param_set()` (or both `test_with_*` methods for `AssertionHandler` subclasses). + +3. Import it in `__init__.py` and add an instance to `ASSERTION_HANDLERS`. + +4. Run `uv run python -m src.babel_validation.assertions.gen_docs` to regenerate `README.md`. diff --git a/src/babel_validation/assertions/__init__.py b/src/babel_validation/assertions/__init__.py new file mode 100644 index 00000000..4ac70edc --- /dev/null +++ b/src/babel_validation/assertions/__init__.py @@ -0,0 +1,187 @@ +""" +babel_validation.assertions +=========================== + +This package defines the assertion types that can be embedded in GitHub issue bodies +and evaluated against the NodeNorm and NameRes services. + +Supported assertion types are registered in ASSERTION_HANDLERS. To see everything +that is currently supported, scan that dict or read assertions/README.md (auto-generated). + +Adding a new assertion type +--------------------------- +1. Create a subclass of NodeNormTest or NameResTest (or AssertionHandler for both) + in the appropriate module (nodenorm.py, nameres.py, or common.py). +2. Set NAME and DESCRIPTION class attributes. +3. Set PARAMETERS, WIKI_EXAMPLES, and YAML_PARAMS class attributes for documentation. +4. Override test_param_set(). +5. Import it here and add an instance to ASSERTION_HANDLERS. +6. Run `uv run python -m src.babel_validation.assertions.gen_docs` to regenerate README.md. +""" + +import re +from typing import Iterator + +from src.babel_validation.core.testrow import TestResult, TestStatus + + +class AssertionHandler: + """Base class for all BabelTest assertion handlers.""" + NAME: str # lowercase assertion name as used in issue bodies + DESCRIPTION: str # one-line human-readable description + + def passed(self, message: str) -> TestResult: + return TestResult(status=TestStatus.Passed, message=message) + + def failed(self, message: str) -> TestResult: + return TestResult(status=TestStatus.Failed, message=message) + + def test_with_nodenorm(self, param_sets: list[list[str]], nodenorm, + label: str = "") -> Iterator[TestResult]: + """Evaluate this assertion against NodeNorm. Returns [] if not applicable. + + :param param_sets: list[list[str]] — see github_issues_test_cases.py module + docstring for the full definition of param_set / param_sets. + """ + return [] + + def test_with_nameres(self, param_sets: list[list[str]], nodenorm, nameres, + pass_if_found_in_top: int = 5, + label: str = "") -> Iterator[TestResult]: + """Evaluate this assertion against NameRes. Returns [] if not applicable. + + :param param_sets: list[list[str]] — see github_issues_test_cases.py module + docstring for the full definition of param_set / param_sets. + """ + return [] + + +class NodeNormTest(AssertionHandler): + """Base class for assertions that test NodeNorm. + + Subclasses implement test_param_set() instead of test_with_nodenorm(). + """ + + _CURIE_RE = re.compile(r'^[A-Za-z][A-Za-z0-9._-]*:[^\s]+$') + + def curie_params(self, params: list[str]) -> list[str]: + """Return the subset of params that are CURIEs (for prewarming and validation). + Default: all params are CURIEs. Subclasses override when some params are non-CURIEs.""" + return params + + def test_with_nodenorm(self, param_sets: list[list[str]], nodenorm, + label: str = "") -> Iterator[TestResult]: + if not param_sets: + yield self.failed(f"No parameters provided in {label}") + return + # Validate each param_set up front so malformed CURIEs are never sent to + # NodeNorm — not even in the cache-warming call below. + failures: dict[int, TestResult] = {} + for index, params in enumerate(param_sets): + if not params: + failures[index] = self.failed(f"No parameters in param_set {index} in {label}") + continue + invalid = [c for c in self.curie_params(params) if not self._CURIE_RE.match(c)] + if invalid: + failures[index] = self.failed( + f"Malformed CURIE(s) {invalid} in param_set {index} in {label}: " + f"expected format PREFIX:LOCAL_ID (e.g. CHEBI:15365)" + ) + # warm the cache only for params that are CURIEs (deduplicated); skip if empty + # (normalize_curies raises ValueError on an empty list) + curies_to_warm = list({ + p + for index, params in enumerate(param_sets) + if index not in failures + for p in self.curie_params(params) + }) + if curies_to_warm: + nodenorm.normalize_curies(curies_to_warm) + results = [] + for index, params in enumerate(param_sets): + if index in failures: + results.append(failures[index]) + continue + results.extend(self.test_param_set(params, nodenorm, label)) + if not results: + yield self.failed(f"No test results returned in {label}") + return + yield from results + + def test_param_set(self, params: list[str], nodenorm, label: str = "") -> Iterator[TestResult]: + """Override this to implement the assertion. Called once per param_set. + + :param params: A single param_set — one element of the outer param_sets list. + See github_issues_test_cases.py for the full terminology. + """ + raise NotImplementedError + + @staticmethod + def first_type(result: dict) -> str: + """First Biolink type of a resolved node, or a placeholder if the node has none. + + NodeNorm normally returns a non-empty `type` list, but guard against an empty + (or missing) one so message formatting never raises IndexError/KeyError.""" + types = result.get('type') or [] + return types[0] if types else 'unknown type' + + def resolved_message(self, curie: str, result: dict, nodenorm) -> str: + """Standard pass-message when a CURIE resolves.""" + return (f"Resolved {curie} to {result['id']['identifier']} " + f"({self.first_type(result)}, \"{result['id'].get('label', '')}\") " + f"with NodeNormalization service {nodenorm}") + + +class NameResTest(AssertionHandler): + """Base class for assertions that test NameRes. + + Subclasses implement test_param_set() instead of test_with_nameres(). + """ + + def test_with_nameres(self, param_sets: list[list[str]], nodenorm, nameres, + pass_if_found_in_top: int = 5, + label: str = "") -> Iterator[TestResult]: + if not param_sets: + yield self.failed(f"No parameters provided in {label}") + return + results = [] + for index, params in enumerate(param_sets): + if not params: + results.append(self.failed(f"No parameters in param_set {index} in {label}")) + continue + results.extend(self.test_param_set(params, nodenorm, nameres, pass_if_found_in_top, label)) + if not results: + yield self.failed(f"No test results returned in {label}") + return + yield from results + + def test_param_set(self, params: list[str], nodenorm, nameres, + pass_if_found_in_top: int, label: str = "") -> Iterator[TestResult]: + """Override this to implement the assertion. Called once per param_set. + + :param params: A single param_set — one element of the outer param_sets list. + See github_issues_test_cases.py for the full terminology. + """ + raise NotImplementedError + + +# Registry — import submodules after base classes are defined to avoid circular imports. +from src.babel_validation.assertions.nodenorm import ( # noqa: E402 + ResolvesHandler, DoesNotResolveHandler, ResolvesWithHandler, + ResolvesWithTypeHandler, DoesNotResolveWithHandler, HasLabelHandler, +) +from src.babel_validation.assertions.nameres import SearchByNameHandler # noqa: E402 +from src.babel_validation.assertions.common import NeededHandler # noqa: E402 + +ASSERTION_HANDLERS: dict[str, AssertionHandler] = { + h.NAME: h for h in [ + ResolvesHandler(), + DoesNotResolveHandler(), + ResolvesWithHandler(), + DoesNotResolveWithHandler(), + HasLabelHandler(), + ResolvesWithTypeHandler(), + SearchByNameHandler(), + NeededHandler(), + ] +} diff --git a/src/babel_validation/assertions/common.py b/src/babel_validation/assertions/common.py new file mode 100644 index 00000000..68c5db22 --- /dev/null +++ b/src/babel_validation/assertions/common.py @@ -0,0 +1,13 @@ +from src.babel_validation.assertions import AssertionHandler + + +class NeededHandler(AssertionHandler): + """Placeholder assertion indicating that a test still needs to be written for this issue.""" + NAME = "needed" + DESCRIPTION = "Marks an issue as needing a test — always fails as a reminder to add real assertions." + PARAMETERS = "" + WIKI_EXAMPLES = ["{{BabelTest|Needed}}"] + YAML_PARAMS = " - placeholder" + + def test_with_nodenorm(self, param_sets, nodenorm, label=""): + yield self.failed("Test needed for issue") diff --git a/src/babel_validation/assertions/gen_docs.py b/src/babel_validation/assertions/gen_docs.py new file mode 100644 index 00000000..c5a77a43 --- /dev/null +++ b/src/babel_validation/assertions/gen_docs.py @@ -0,0 +1,146 @@ +"""Generate assertions/README.md from handler class attributes. + +Run: + uv run python -m src.babel_validation.assertions.gen_docs +""" + +from pathlib import Path + +from src.babel_validation.assertions import ( + ASSERTION_HANDLERS, AssertionHandler, NodeNormTest, NameResTest, +) + +README_PATH = Path(__file__).parent / "README.md" + +INTRO = """\ + + +# BabelTest Assertion Types + +This package defines the assertion types that can be embedded in GitHub issue bodies and evaluated against the NodeNorm and NameRes services. + +## Embedding Tests in Issues + +Two syntaxes are supported: + +**Wiki syntax** (one assertion per line): +``` +{{BabelTest|AssertionType|param1|param2|...}} +``` + +**YAML syntax** (multiple assertions, multiple param sets): +```` +```yaml +babel_tests: + AssertionType: + - param1 + - [param1, param2] +``` +```` + +Assertion names are case-insensitive, as is the `{{BabelTest|...}}` marker itself. + +## Param Sets + +Each assertion can be invoked with one or more **param sets** — independent groups of +parameters that are each evaluated separately. + +- **Wiki syntax** — each `{{BabelTest|...}}` line is one param set. +- **YAML syntax** — each list entry under an assertion key is one param set; a bare string + is a single-element param set, a YAML list is a multi-element param set. + +The meaning of each element in a param set depends on the assertion type (see below). +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. + +--- +""" + +ADDING_NEW = """\ +## Adding a New Assertion Type + +1. Choose the right module: + - `nodenorm.py` — for NodeNorm-only assertions (subclass `NodeNormTest`, override `test_param_set`) + - `nameres.py` — for NameRes-only assertions (subclass `NameResTest`, override `test_param_set`) + - `common.py` — for assertions that apply to both services (subclass `AssertionHandler`, override `test_with_nodenorm` and/or `test_with_nameres`) + +2. Define the class with `NAME`, `DESCRIPTION`, `PARAMETERS`, `WIKI_EXAMPLES`, `YAML_PARAMS`, and `test_param_set()` (or both `test_with_*` methods for `AssertionHandler` subclasses). + +3. Import it in `__init__.py` and add an instance to `ASSERTION_HANDLERS`. + +4. Run `uv run python -m src.babel_validation.assertions.gen_docs` to regenerate `README.md`. +""" + +_GROUP_HEADERS: dict[str, str] = { + "NodeNorm": ( + "## NodeNorm Assertions\n\n" + "These assertions test the [NodeNorm](https://nodenorm.transltr.io/docs) service." + ), + "NameRes": ( + "## NameRes Assertions\n\n" + "These assertions test the [NameRes](https://name-lookup.transltr.io/docs) service." + ), + "NodeNorm and NameRes": "## Special Assertions", +} + + +def _display_name(h: AssertionHandler) -> str: + return type(h).__name__.removesuffix("Handler") + + +def _applies_to(h: AssertionHandler) -> str: + if isinstance(h, NodeNormTest): + return "NodeNorm" + if isinstance(h, NameResTest): + return "NameRes" + return "NodeNorm and NameRes" + + +def _render_handler(h: AssertionHandler) -> str: + name = _display_name(h) + service = _applies_to(h) + description = getattr(h, "DESCRIPTION", "") + parameters = getattr(h, "PARAMETERS", "") + wiki_examples = getattr(h, "WIKI_EXAMPLES", []) + yaml_params = getattr(h, "YAML_PARAMS", "") + + parts = [] + parts.append(f"### {name}\n") + parts.append(f"**Applies to:** {service}\n") + parts.append(f"{description}\n") + + if parameters: + parts.append(f"**Parameters:** {parameters}\n") + + wiki_block = "\n".join(wiki_examples) + parts.append(f"**Wiki syntax:**\n```\n{wiki_block}\n```\n") + + parts.append( + f"**YAML syntax:**\n```yaml\nbabel_tests:\n {name}:\n{yaml_params}\n```\n" + ) + + parts.append("---\n") + + return "\n".join(parts) + + +def generate_readme() -> str: + sections = [INTRO] + seen_groups: set[str] = set() + + for h in ASSERTION_HANDLERS.values(): + service = _applies_to(h) + if service not in seen_groups: + seen_groups.add(service) + sections.append(_GROUP_HEADERS[service] + "\n") + sections.append(_render_handler(h)) + + sections.append(ADDING_NEW) + return "\n".join(sections) + + +if __name__ == "__main__": + content = generate_readme() + README_PATH.write_text(content, encoding="utf-8") + print(f"Written to {README_PATH}") diff --git a/src/babel_validation/assertions/nameres.py b/src/babel_validation/assertions/nameres.py new file mode 100644 index 00000000..da4cb390 --- /dev/null +++ b/src/babel_validation/assertions/nameres.py @@ -0,0 +1,61 @@ +import json +import logging +from typing import Iterator + +from src.babel_validation.assertions import NameResTest +from src.babel_validation.core.testrow import TestResult +from src.babel_validation.services.nameres import CachedNameRes +from src.babel_validation.services.nodenorm import CachedNodeNorm + + +class SearchByNameHandler(NameResTest): + """Test that a name search returns an expected CURIE in the top-N results in NameRes.""" + NAME = "searchbyname" + DESCRIPTION = ( + "Each param_set must have at least two elements: a search query string and an expected CURIE. " + "The test passes if the CURIE's normalized identifier appears within the top N results " + "(default N=5) when NameRes looks up the search query." + ) + PARAMETERS = ( + "Each param_set: the **search query string** and the **expected CURIE**. " + "The CURIE is normalized via NodeNorm (drug/chemical conflation enabled) before matching." + ) + WIKI_EXAMPLES = ["{{BabelTest|SearchByName|water|CHEBI:15377}}"] + YAML_PARAMS = " - [water, CHEBI:15377]\n - [diabetes, MONDO:0005015]" + + def test_param_set(self, params: list[str], nodenorm: CachedNodeNorm, + nameres: CachedNameRes, pass_if_found_in_top: int = 5, + label: str = "") -> Iterator[TestResult]: + if len(params) != 2: + yield self.failed( + f"SearchByName requires exactly two parameters (search query, expected CURIE) in {label}, " + f"but got {len(params)}: {params}" + ) + return + + [search_query, expected_curie_from_test] = params + expected_curie_result = nodenorm.normalize_curie(expected_curie_from_test, drug_chemical_conflate='true') + if not expected_curie_result: + yield self.failed(f"Unable to normalize CURIE {expected_curie_from_test} in {label}") + return + + expected_curie = expected_curie_result['id']['identifier'] + expected_curie_label = expected_curie_result['id']['label'] + expected_curie_string = f"Expected CURIE {expected_curie_from_test}, normalized to {expected_curie} '{expected_curie_label}'" + + results = nameres.lookup(search_query, autocomplete='false', limit=pass_if_found_in_top) + if not results: + yield self.failed(f"No results found for '{search_query}' on NameRes {nameres} ({expected_curie_string})") + return + + curies = [result['curie'] for result in results] + if expected_curie not in curies: + logging.getLogger(__name__).debug( + "%s not found in top %d results for '%s' in NameRes %s: %s", + expected_curie_string, pass_if_found_in_top, search_query, nameres, + json.dumps(results, indent=2, sort_keys=True) + ) + yield self.failed(f"{expected_curie_string} not found in top {pass_if_found_in_top} results for '{search_query}' in NameRes {nameres}") + return + + yield self.passed(f"{expected_curie_string} found at index {curies.index(expected_curie) + 1} on NameRes {nameres}") diff --git a/src/babel_validation/assertions/nodenorm.py b/src/babel_validation/assertions/nodenorm.py new file mode 100644 index 00000000..bcd010a3 --- /dev/null +++ b/src/babel_validation/assertions/nodenorm.py @@ -0,0 +1,249 @@ +from typing import Iterator + +from src.babel_validation.assertions import NodeNormTest +from src.babel_validation.core.testrow import TestResult +from src.babel_validation.services.nodenorm import CachedNodeNorm + + +class ResolvesHandler(NodeNormTest): + """Test that every CURIE in every param_set resolves in NodeNorm.""" + NAME = "resolves" + DESCRIPTION = "Each CURIE in each param_set must resolve to a non-null result in NodeNorm." + PARAMETERS = "One or more CURIEs per param_set." + WIKI_EXAMPLES = [ + "{{BabelTest|Resolves|CHEBI:15365}}", + "{{BabelTest|Resolves|MONDO:0005015|DOID:9351}}", + ] + YAML_PARAMS = " - CHEBI:15365\n - [MONDO:0005015, DOID:9351]" + + def test_param_set(self, params: list[str], nodenorm: CachedNodeNorm, + label: str = "") -> Iterator[TestResult]: + for curie in params: + result = nodenorm.normalize_curie(curie) + if not result: + yield self.failed(f"Could not resolve {curie} with NodeNormalization service {nodenorm}") + else: + yield self.passed(self.resolved_message(curie, result, nodenorm)) + + +class DoesNotResolveHandler(NodeNormTest): + """Test that every CURIE in every param_set does NOT resolve in NodeNorm.""" + NAME = "doesnotresolve" + DESCRIPTION = ( + "Each CURIE in each param_set must fail to resolve (return null) in NodeNorm. " + "Use this to confirm that an identifier is intentionally not normalizable." + ) + PARAMETERS = "One or more CURIEs per param_set." + WIKI_EXAMPLES = ["{{BabelTest|DoesNotResolve|FAKENS:99999}}"] + YAML_PARAMS = " - FAKENS:99999" + + def test_param_set(self, params: list[str], nodenorm: CachedNodeNorm, + label: str = "") -> Iterator[TestResult]: + for curie in params: + result = nodenorm.normalize_curie(curie) + if not result: + yield self.passed(f"Could not resolve {curie} with NodeNormalization service {nodenorm} as expected") + else: + yield self.failed(f"Resolved {curie} to {result['id']['identifier']} ({self.first_type(result)}, \"{result['id'].get('label', '')}\") with NodeNormalization service {nodenorm}, but expected not to resolve") + + +def _compare_resolutions( + params: list[str], nodenorm: CachedNodeNorm +) -> tuple[dict | None, dict[str, dict | None]]: + """Resolve all params; return (first_good_result, per_curie_results). + + first_good_result is None if every CURIE failed to resolve. + per_curie_results maps each CURIE to its result (None if unresolvable). + """ + per_curie = nodenorm.normalize_curies(params) + first_good = next((r for r in per_curie.values() if r is not None), None) + return first_good, per_curie + + +class ResolvesWithHandler(NodeNormTest): + """Test that all CURIEs in a param_set resolve to the same normalized result in NodeNorm.""" + NAME = "resolveswith" + DESCRIPTION = ( + "All CURIEs within each param_set must resolve to the identical normalized result. " + "Use this to assert that two identifiers are equivalent." + ) + PARAMETERS = "Two or more CURIEs per param_set. All must resolve to the same result." + WIKI_EXAMPLES = ["{{BabelTest|ResolvesWith|CHEBI:15365|PUBCHEM.COMPOUND:1}}"] + YAML_PARAMS = " - [CHEBI:15365, PUBCHEM.COMPOUND:1]\n - [MONDO:0005015, DOID:9351]" + + def test_param_set(self, params: list[str], nodenorm: CachedNodeNorm, + label: str = "") -> Iterator[TestResult]: + if len(params) < 2: + yield self.failed( + f"ResolvesWith requires at least two CURIEs per param_set in {label}, " + f"but got {len(params)}: {params}" + ) + return + + first_good, results = _compare_resolutions(params, nodenorm) + + if first_good is None: + yield self.failed(f"None of the CURIEs {params} could be resolved on {nodenorm}") + return + + canonical_id = first_good['id']['identifier'] + + for curie, result in results.items(): + if result is None: + yield self.failed( + f"CURIE {curie} could not be resolved on {nodenorm}" + ) + elif result['id']['identifier'] == canonical_id: + yield self.passed( + f"Resolved {curie} to the expected canonical identifier {canonical_id}" + ) + else: + yield self.failed( + f"Resolved {curie} to {result['id']['identifier']} " + f"({self.first_type(result)}, \"{result['id'].get('label', '')}\"), but expected " + f"{canonical_id} " + f"({self.first_type(first_good)}, \"{first_good['id'].get('label', '')}\") on {nodenorm}" + ) + + +class DoesNotResolveWithHandler(NodeNormTest): + """Test that not all CURIEs in a param_set resolve to the same result in NodeNorm.""" + NAME = "doesnotresolvewith" + DESCRIPTION = ( + "The CURIEs within each param_set must NOT all resolve to the same normalized " + "result. Use this to assert that two identifiers are intentionally distinct entities." + ) + PARAMETERS = "Two or more CURIEs per param_set. They must not all resolve to the same result." + WIKI_EXAMPLES = ["{{BabelTest|DoesNotResolveWith|CHEBI:15365|CHEBI:16856}}"] + YAML_PARAMS = " - [CHEBI:15365, CHEBI:16856]" + + def test_param_set(self, params: list[str], nodenorm: CachedNodeNorm, + label: str = "") -> Iterator[TestResult]: + if len(params) < 2: + yield self.failed( + f"DoesNotResolveWith requires at least two CURIEs per param_set in {label}, " + f"but got {len(params)}: {params}" + ) + return + + first_good, results = _compare_resolutions(params, nodenorm) + + # Every CURIE must resolve — an unresolved CURIE is a configuration error. + unresolved = [curie for curie, result in results.items() if result is None] + if unresolved: + yield self.failed( + f"CURIEs {unresolved} could not be resolved on {nodenorm}; " + f"all CURIEs in a DoesNotResolveWith param_set must resolve" + ) + return + + # All resolved — check that they don't all map to the same canonical identifier. + canonical_ids = {result['id']['identifier'] for result in results.values()} + + if len(canonical_ids) == 1: + # Every CURIE maps to the same result — assertion fails. + shared = first_good + yield self.failed( + f"All CURIEs {params} resolved to the same result " + f"{shared['id']['identifier']} " + f"({self.first_type(shared)}, \"{shared['id'].get('label', '')}\") on {nodenorm}, " + f"but expected them to resolve differently" + ) + else: + summary = ", ".join( + f"{curie} → {result['id']['identifier']}" + for curie, result in results.items() + ) + yield self.passed( + f"CURIEs resolve to different results as expected: {summary} on {nodenorm}" + ) + + +class HasLabelHandler(NodeNormTest): + """Test that a CURIE resolves to a specific primary label in NodeNorm.""" + NAME = "haslabel" + DESCRIPTION = ( + "The CURIE must resolve in NodeNorm and its primary label (id.label) must " + "match the expected label exactly (case-sensitive)." + ) + PARAMETERS = "Exactly two elements per param_set: a CURIE, then the expected label string." + WIKI_EXAMPLES = ["{{BabelTest|HasLabel|CHEBI:15365|aspirin}}"] + YAML_PARAMS = " - [CHEBI:15365, aspirin]" + + def curie_params(self, params: list[str]) -> list[str]: + return params[:1] + + def test_param_set(self, params: list[str], nodenorm: CachedNodeNorm, + label: str = "") -> Iterator[TestResult]: + if len(params) != 2: + yield self.failed( + f"HasLabel requires exactly two parameters (CURIE, expected label) in {label}, " + f"but got {len(params)}: {params}" + ) + return + + curie = params[0] + expected_label = params[1].strip() + + result = nodenorm.normalize_curie(curie) + if not result: + yield self.failed( + f"Could not resolve {curie} on {nodenorm}" + ) + return + + if 'label' not in result['id']: + yield self.failed( + f"CURIE {curie} has no label but expected '{expected_label}' on {nodenorm}" + ) + return + + actual_label = result['id']['label'] + if actual_label == expected_label: + yield self.passed( + f"CURIE {curie} has expected label '{actual_label}' on {nodenorm}" + ) + else: + yield self.failed( + f"CURIE {curie} has label '{actual_label}', " + f"but expected '{expected_label}' on {nodenorm}" + ) + + +class ResolvesWithTypeHandler(NodeNormTest): + """Test that CURIEs resolve with a specific Biolink type in NodeNorm.""" + NAME = "resolveswithtype" + DESCRIPTION = ( + "Each param_set must have at least two elements: the first is the expected Biolink type " + "(e.g. 'biolink:Gene'), and the remainder are CURIEs that must resolve with that type." + ) + PARAMETERS = ( + "Each param_set: first element is the expected Biolink type (e.g. `biolink:Gene`), " + "remaining elements are CURIEs." + ) + WIKI_EXAMPLES = ["{{BabelTest|ResolvesWithType|biolink:Gene|NCBIGene:1}}"] + YAML_PARAMS = " - [biolink:Gene, NCBIGene:1, HGNC:5]" + + def curie_params(self, params: list[str]) -> list[str]: + return params[1:] + + def test_param_set(self, params: list[str], nodenorm: CachedNodeNorm, + label: str = "") -> Iterator[TestResult]: + if len(params) < 2: + yield self.failed(f"Too few parameters provided in param_set in {label}: {params}") + return + + expected_biolink_type = params[0] + curies = params[1:] + + results = nodenorm.normalize_curies(curies) + for curie in curies: + node = results.get(curie) + if not node: + yield self.failed(f"Could not resolve {curie} with NodeNormalization service {nodenorm}") + continue + biolink_types = node['type'] + if expected_biolink_type in biolink_types: + yield self.passed(f"Biolink types {biolink_types} for CURIE {curie} includes expected Biolink type {expected_biolink_type}") + else: + yield self.failed(f"Biolink types {biolink_types} for CURIE {curie} does not include expected Biolink type {expected_biolink_type}") diff --git a/tests/test_environment/test_assertions_docs.py b/tests/test_environment/test_assertions_docs.py new file mode 100644 index 00000000..03e5b908 --- /dev/null +++ b/tests/test_environment/test_assertions_docs.py @@ -0,0 +1,14 @@ +import pytest + +from src.babel_validation.assertions.gen_docs import generate_readme, README_PATH + + +@pytest.mark.unit +def test_assertions_readme_is_up_to_date(): + expected = generate_readme() + actual = README_PATH.read_text(encoding="utf-8").replace("\r\n", "\n") + assert actual == expected, ( + "assertions/README.md is out of date.\n" + "Regenerate it with:\n" + " uv run python -m src.babel_validation.assertions.gen_docs" + ) From 3bf8587f8ea30ab5f8b4816b54ebf85be7c55242 Mon Sep 17 00:00:00 2001 From: Gaurav Vaidya Date: Fri, 26 Jun 2026 14:57:42 -0400 Subject: [PATCH 02/23] Fix crash guard in ResolvesWithTypeHandler and NeededHandler NameRes path - ResolvesWithTypeHandler: use node.get('type') or [] instead of node['type'] to match the existing first_type() guard (NodeNorm can return results without a type key) - NeededHandler: add test_with_nameres override so Needed issues fail on both NodeNorm and NameRes paths, not just NodeNorm Co-Authored-By: Claude Sonnet 4.6 --- src/babel_validation/assertions/common.py | 3 +++ src/babel_validation/assertions/nodenorm.py | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/src/babel_validation/assertions/common.py b/src/babel_validation/assertions/common.py index 68c5db22..ac12daca 100644 --- a/src/babel_validation/assertions/common.py +++ b/src/babel_validation/assertions/common.py @@ -11,3 +11,6 @@ class NeededHandler(AssertionHandler): def test_with_nodenorm(self, param_sets, nodenorm, label=""): yield self.failed("Test needed for issue") + + def test_with_nameres(self, param_sets, nodenorm, nameres, pass_if_found_in_top=5, label=""): + yield self.failed("Test needed for issue") diff --git a/src/babel_validation/assertions/nodenorm.py b/src/babel_validation/assertions/nodenorm.py index bcd010a3..92f477f5 100644 --- a/src/babel_validation/assertions/nodenorm.py +++ b/src/babel_validation/assertions/nodenorm.py @@ -242,7 +242,7 @@ def test_param_set(self, params: list[str], nodenorm: CachedNodeNorm, if not node: yield self.failed(f"Could not resolve {curie} with NodeNormalization service {nodenorm}") continue - biolink_types = node['type'] + biolink_types = node.get('type') or [] if expected_biolink_type in biolink_types: yield self.passed(f"Biolink types {biolink_types} for CURIE {curie} includes expected Biolink type {expected_biolink_type}") else: From 381d815e25d0a8a6e86c3abcba92e8fb731c78c5 Mon Sep 17 00:00:00 2001 From: Gaurav Vaidya Date: Fri, 26 Jun 2026 14:57:49 -0400 Subject: [PATCH 03/23] Fix SearchByName: drop conflation override and guard missing label - Remove drug_chemical_conflate='true' from the NodeNorm call used to canonicalize the expected CURIE; NameRes normalizes without conflation, so using it caused structural false failures for drug/chemical CURIEs - Use .get('label', '') instead of ['label'] to avoid KeyError for CURIEs that NodeNorm resolves without a preferred label - Regenerate assertions/README.md to reflect the updated parameter description Co-Authored-By: Claude Sonnet 4.6 --- src/babel_validation/assertions/README.md | 2 +- src/babel_validation/assertions/nameres.py | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/babel_validation/assertions/README.md b/src/babel_validation/assertions/README.md index 9cc4ef7d..9f63cf41 100644 --- a/src/babel_validation/assertions/README.md +++ b/src/babel_validation/assertions/README.md @@ -190,7 +190,7 @@ These assertions test the [NameRes](https://name-lookup.transltr.io/docs) servic Each param_set must have at least two elements: a search query string and an expected CURIE. The test passes if the CURIE's normalized identifier appears within the top N results (default N=5) when NameRes looks up the search query. -**Parameters:** Each param_set: the **search query string** and the **expected CURIE**. The CURIE is normalized via NodeNorm (drug/chemical conflation enabled) before matching. +**Parameters:** Each param_set: the **search query string** and the **expected CURIE**. The CURIE is normalized via NodeNorm before matching. **Wiki syntax:** ``` diff --git a/src/babel_validation/assertions/nameres.py b/src/babel_validation/assertions/nameres.py index da4cb390..8bd2bd58 100644 --- a/src/babel_validation/assertions/nameres.py +++ b/src/babel_validation/assertions/nameres.py @@ -18,7 +18,7 @@ class SearchByNameHandler(NameResTest): ) PARAMETERS = ( "Each param_set: the **search query string** and the **expected CURIE**. " - "The CURIE is normalized via NodeNorm (drug/chemical conflation enabled) before matching." + "The CURIE is normalized via NodeNorm before matching." ) WIKI_EXAMPLES = ["{{BabelTest|SearchByName|water|CHEBI:15377}}"] YAML_PARAMS = " - [water, CHEBI:15377]\n - [diabetes, MONDO:0005015]" @@ -34,13 +34,13 @@ def test_param_set(self, params: list[str], nodenorm: CachedNodeNorm, return [search_query, expected_curie_from_test] = params - expected_curie_result = nodenorm.normalize_curie(expected_curie_from_test, drug_chemical_conflate='true') + expected_curie_result = nodenorm.normalize_curie(expected_curie_from_test) if not expected_curie_result: yield self.failed(f"Unable to normalize CURIE {expected_curie_from_test} in {label}") return expected_curie = expected_curie_result['id']['identifier'] - expected_curie_label = expected_curie_result['id']['label'] + expected_curie_label = expected_curie_result['id'].get('label', '') expected_curie_string = f"Expected CURIE {expected_curie_from_test}, normalized to {expected_curie} '{expected_curie_label}'" results = nameres.lookup(search_query, autocomplete='false', limit=pass_if_found_in_top) From a23c07c73136b5e6026461bcbb539f58c27ae128 Mon Sep 17 00:00:00 2001 From: Gaurav Vaidya Date: Fri, 26 Jun 2026 15:50:44 -0400 Subject: [PATCH 04/23] Address Copilot review: fix docstrings and return type in base AssertionHandler - AssertionHandler.test_with_nodenorm/test_with_nameres: return iter([]) instead of [] to match the Iterator[TestResult] annotation; simplify docstrings to remove reference to github_issues_test_cases.py (module not yet in this PR) - Remove same stale reference from NodeNormTest.test_param_set and NameResTest.test_param_set docstrings - SearchByNameHandler.DESCRIPTION: "exactly two" not "at least two" to match the len(params) != 2 enforcement in test_param_set - Regenerate assertions/README.md Co-Authored-By: Claude Sonnet 4.6 --- src/babel_validation/assertions/README.md | 2 +- src/babel_validation/assertions/__init__.py | 28 +++++---------------- src/babel_validation/assertions/nameres.py | 2 +- 3 files changed, 8 insertions(+), 24 deletions(-) diff --git a/src/babel_validation/assertions/README.md b/src/babel_validation/assertions/README.md index 9f63cf41..65929d1e 100644 --- a/src/babel_validation/assertions/README.md +++ b/src/babel_validation/assertions/README.md @@ -188,7 +188,7 @@ These assertions test the [NameRes](https://name-lookup.transltr.io/docs) servic **Applies to:** NameRes -Each param_set must have at least two elements: a search query string and an expected CURIE. The test passes if the CURIE's normalized identifier appears within the top N results (default N=5) when NameRes looks up the search query. +Each param_set must have exactly two elements: a search query string and an expected CURIE. The test passes if the CURIE's normalized identifier appears within the top N results (default N=5) when NameRes looks up the search query. **Parameters:** Each param_set: the **search query string** and the **expected CURIE**. The CURIE is normalized via NodeNorm before matching. diff --git a/src/babel_validation/assertions/__init__.py b/src/babel_validation/assertions/__init__.py index 4ac70edc..ae82b4c1 100644 --- a/src/babel_validation/assertions/__init__.py +++ b/src/babel_validation/assertions/__init__.py @@ -38,22 +38,14 @@ def failed(self, message: str) -> TestResult: def test_with_nodenorm(self, param_sets: list[list[str]], nodenorm, label: str = "") -> Iterator[TestResult]: - """Evaluate this assertion against NodeNorm. Returns [] if not applicable. - - :param param_sets: list[list[str]] — see github_issues_test_cases.py module - docstring for the full definition of param_set / param_sets. - """ - return [] + """Evaluate this assertion against NodeNorm. Returns nothing if not applicable.""" + return iter([]) def test_with_nameres(self, param_sets: list[list[str]], nodenorm, nameres, pass_if_found_in_top: int = 5, label: str = "") -> Iterator[TestResult]: - """Evaluate this assertion against NameRes. Returns [] if not applicable. - - :param param_sets: list[list[str]] — see github_issues_test_cases.py module - docstring for the full definition of param_set / param_sets. - """ - return [] + """Evaluate this assertion against NameRes. Returns nothing if not applicable.""" + return iter([]) class NodeNormTest(AssertionHandler): @@ -109,11 +101,7 @@ def test_with_nodenorm(self, param_sets: list[list[str]], nodenorm, yield from results def test_param_set(self, params: list[str], nodenorm, label: str = "") -> Iterator[TestResult]: - """Override this to implement the assertion. Called once per param_set. - - :param params: A single param_set — one element of the outer param_sets list. - See github_issues_test_cases.py for the full terminology. - """ + """Override this to implement the assertion. Called once per param_set.""" raise NotImplementedError @staticmethod @@ -157,11 +145,7 @@ def test_with_nameres(self, param_sets: list[list[str]], nodenorm, nameres, def test_param_set(self, params: list[str], nodenorm, nameres, pass_if_found_in_top: int, label: str = "") -> Iterator[TestResult]: - """Override this to implement the assertion. Called once per param_set. - - :param params: A single param_set — one element of the outer param_sets list. - See github_issues_test_cases.py for the full terminology. - """ + """Override this to implement the assertion. Called once per param_set.""" raise NotImplementedError diff --git a/src/babel_validation/assertions/nameres.py b/src/babel_validation/assertions/nameres.py index 8bd2bd58..a0b493f0 100644 --- a/src/babel_validation/assertions/nameres.py +++ b/src/babel_validation/assertions/nameres.py @@ -12,7 +12,7 @@ class SearchByNameHandler(NameResTest): """Test that a name search returns an expected CURIE in the top-N results in NameRes.""" NAME = "searchbyname" DESCRIPTION = ( - "Each param_set must have at least two elements: a search query string and an expected CURIE. " + "Each param_set must have exactly two elements: a search query string and an expected CURIE. " "The test passes if the CURIE's normalized identifier appears within the top N results " "(default N=5) when NameRes looks up the search query." ) From 9fedae666e956fa394455e2fa50f1d4e2d86cd39 Mon Sep 17 00:00:00 2001 From: Gaurav Vaidya Date: Fri, 26 Jun 2026 13:52:45 -0400 Subject: [PATCH 05/23] Fix two Copilot-flagged issues in google_sheet_test_cases.py - Switch absolute src.babel_validation import to a relative import (..core.testrow) so internal modules aren't coupled to the top-level packaging layout. - Use full MD5 digest for cache filename instead of truncating to 8 chars, eliminating the theoretical hash-collision risk across different sheet IDs. Co-Authored-By: Claude Sonnet 4.6 --- .../sources/google_sheets/google_sheet_test_cases.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/babel_validation/sources/google_sheets/google_sheet_test_cases.py b/src/babel_validation/sources/google_sheets/google_sheet_test_cases.py index 935b019e..ccd71252 100644 --- a/src/babel_validation/sources/google_sheets/google_sheet_test_cases.py +++ b/src/babel_validation/sources/google_sheets/google_sheet_test_cases.py @@ -15,7 +15,7 @@ from _pytest.mark import ParameterSet from filelock import FileLock -from src.babel_validation.core.testrow import TestRow +from ...core.testrow import TestRow class GoogleSheetTestCases: @@ -39,7 +39,7 @@ def __init__(self, google_sheet_id="11zebx8Qs1Tc3ShQR9nh4HRW8QSoo8k65w_xIaftN0no self.google_sheet_id = google_sheet_id - sheet_hash = hashlib.md5(google_sheet_id.encode()).hexdigest()[:8] + sheet_hash = hashlib.md5(google_sheet_id.encode()).hexdigest() cache_file = Path(tempfile.gettempdir()) / f"babel_validation_gsheet_{sheet_hash}.csv" lock_file = cache_file.with_suffix(".lock") From 0c3a717537c001fb341226a1f8bc307a3ca767be Mon Sep 17 00:00:00 2001 From: Gaurav Vaidya Date: Fri, 26 Jun 2026 13:57:07 -0400 Subject: [PATCH 06/23] Renamed _silent_unlink() to the more sensible unlink_if_exists(). --- tests/conftest.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 96e75f72..8c768ab7 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -28,7 +28,13 @@ def get_targets_ini_path(config): return config_path -def _silent_unlink(path: str) -> None: +def unlink_if_exists(path: str) -> None: + """ + Unlink the file at `path` if it exists. + + :param path: The path to the file to unlink. + :return: None + """ try: os.unlink(path) except FileNotFoundError: @@ -41,8 +47,8 @@ def pytest_configure(config): # so they can share the cache file written by the controller. if not os.environ.get('PYTEST_XDIST_WORKER'): for f in glob.glob(os.path.join(tempfile.gettempdir(), 'babel_validation_gsheet_*.csv')): - _silent_unlink(f) - _silent_unlink(f.removesuffix('.csv') + '.lock') + unlink_if_exists(f) + unlink_if_exists(f.removesuffix('.csv') + '.lock') def pytest_addoption(parser): @@ -139,4 +145,4 @@ def category_test(cat): return False return True - return category_test \ No newline at end of file + return category_test From 9ff77c15ab760035632b2bd5d4951eaff798b0c3 Mon Sep 17 00:00:00 2001 From: Gaurav Vaidya Date: Fri, 26 Jun 2026 14:10:31 -0400 Subject: [PATCH 07/23] Make cache_ttl_seconds a constructor parameter in GoogleSheetTestCases Co-Authored-By: Claude Sonnet 4.6 --- .../google_sheets/google_sheet_test_cases.py | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/src/babel_validation/sources/google_sheets/google_sheet_test_cases.py b/src/babel_validation/sources/google_sheets/google_sheet_test_cases.py index ccd71252..e764b958 100644 --- a/src/babel_validation/sources/google_sheets/google_sheet_test_cases.py +++ b/src/babel_validation/sources/google_sheets/google_sheet_test_cases.py @@ -26,15 +26,13 @@ class GoogleSheetTestCases: def __str__(self): return f"Google Sheet Test Cases ({len(self.rows)} test cases from {self.google_sheet_id})" - # How long a cached download stays valid. pytest deletes the cache at the - # start of every run (see tests/conftest.py), so this TTL mainly protects - # other consumers (e.g. csv-to-babeltests) from reading stale data forever. - CACHE_TTL_SECONDS = 3600 - - def __init__(self, google_sheet_id="11zebx8Qs1Tc3ShQR9nh4HRW8QSoo8k65w_xIaftN0no"): + def __init__(self, google_sheet_id="11zebx8Qs1Tc3ShQR9nh4HRW8QSoo8k65w_xIaftN0no", cache_ttl_seconds: int = 3600): """ Create a Google Sheet test case. - :param google_sheet_id The Google Sheet identifier to download test cases from. + :param google_sheet_id: The Google Sheet identifier to download test cases from. + :param cache_ttl_seconds: How long a cached download stays valid. pytest deletes the cache at the + start of every run (see tests/conftest.py), so this TTL mainly protects other consumers + (e.g. csv-to-babeltests) from reading stale data forever. """ self.google_sheet_id = google_sheet_id @@ -44,7 +42,7 @@ def __init__(self, google_sheet_id="11zebx8Qs1Tc3ShQR9nh4HRW8QSoo8k65w_xIaftN0no lock_file = cache_file.with_suffix(".lock") with FileLock(lock_file): - if cache_file.exists() and time.time() - cache_file.stat().st_mtime < self.CACHE_TTL_SECONDS: + if cache_file.exists() and time.time() - cache_file.stat().st_mtime < cache_ttl_seconds: self.csv_content = cache_file.read_text(encoding="utf-8") else: csv_url = f"https://docs.google.com/spreadsheets/d/{google_sheet_id}/gviz/tq?tqx=out:csv&sheet=Tests" From fb724453e16eed300663cf8237493d9409e86fab Mon Sep 17 00:00:00 2001 From: Gaurav Vaidya Date: Fri, 26 Jun 2026 14:22:53 -0400 Subject: [PATCH 08/23] Document caching model and cache-warming pattern in service modules Adds module docstrings explaining the per-(identifier, params) cache key, the no-auto-eviction policy, and the intended cache-warming pattern: call the batch method once for all identifiers in a task, then use the single-item method per assertion at zero HTTP cost. Adds method docstrings to from_url(), the batch methods (normalize_curies / bulk_lookup), the single-item methods (normalize_curie / lookup), and the cache-clearing methods. Includes a note that lookup() targets a distinct NameRes endpoint from bulk_lookup() and does not delegate to it. Co-Authored-By: Claude Sonnet 4.6 --- src/babel_validation/services/nameres.py | 57 +++++++++++++++++++++++ src/babel_validation/services/nodenorm.py | 55 +++++++++++++++++++++- 2 files changed, 110 insertions(+), 2 deletions(-) diff --git a/src/babel_validation/services/nameres.py b/src/babel_validation/services/nameres.py index 01d36bb1..aef6ac2e 100644 --- a/src/babel_validation/services/nameres.py +++ b/src/babel_validation/services/nameres.py @@ -1,3 +1,27 @@ +""" +Cached client for the NameRes ``bulk-lookup`` and ``lookup`` APIs. + +Caching model +------------- +Each response is stored under the key ``(query, frozenset(params.items()))``. +Entries are never evicted automatically; call ``delete_query()`` to force +a fresh lookup for a specific query string. + +Cache-warming pattern +--------------------- +When you need to look up many query strings for the same logical task, call +``bulk_lookup()`` once with the full list. That issues a single HTTP POST to +the ``bulk-lookup`` endpoint and populates the cache. Subsequent +``bulk_lookup()`` calls for any subset of those queries are served from cache. + +Endpoint differences +-------------------- +``bulk_lookup()`` targets ``/bulk-lookup`` and sends the query list as a JSON +body. ``lookup()`` targets the separate ``/lookup`` endpoint and sends its +parameters as a URL query string. These are distinct API endpoints with +different response shapes; ``lookup()`` does NOT delegate to ``bulk_lookup()``. +""" + import logging import time @@ -16,11 +40,29 @@ def __str__(self): @staticmethod def from_url(nameres_url: str) -> 'CachedNameRes': + """Return the singleton ``CachedNameRes`` for *nameres_url*. + + The singleton ensures that cache entries accumulated during one part of + a test run are reused by later parts that share the same URL. Prefer + this over direct construction unless you explicitly want a fresh cache. + """ if nameres_url not in cached_nameres_by_url: cached_nameres_by_url[nameres_url] = CachedNameRes(nameres_url) return cached_nameres_by_url[nameres_url] def bulk_lookup(self, queries: list[str], **params) -> dict[str, dict]: + """Look up *queries* in bulk, returning a ``{query: result}`` mapping. + + Already-cached queries are served from the cache; the remainder are + fetched from NameRes in a single HTTP POST to ``bulk-lookup``. + The response is merged with the cached results before returning. + + *queries* must be a non-empty list — the NameRes API rejects empty + requests, so this method raises ``ValueError`` immediately. + + Use this as the cache-warming call; subsequent ``bulk_lookup()`` calls + for any subset of these queries will be free. + """ if not queries: raise ValueError(f"queries must not be empty when calling bulk_lookup({queries}, {params}) on {self}") if not isinstance(queries, list): @@ -55,6 +97,16 @@ def bulk_lookup(self, queries: list[str], **params) -> dict[str, dict]: return result def lookup(self, query, **params): + """Look up a single *query* string via the NameRes ``/lookup`` endpoint. + + This targets a different endpoint from ``bulk_lookup()`` — parameters + are sent as URL query string fields, and the response is a list of + result dicts rather than a mapping. Results are cached per + ``(query, params)`` combination. + + This method does NOT delegate to ``bulk_lookup()``. To cache-warm for + single lookups, call this method (or ``bulk_lookup()``) upfront. + """ cache_key = (query, frozenset(params.items())) if cache_key in self.cache: return self.cache[cache_key] @@ -71,6 +123,11 @@ def lookup(self, query, **params): return result def delete_query(self, query): + """Remove all cached results for *query* (across every param variant). + + The next call to ``lookup()`` or ``bulk_lookup()`` for this query will + issue a fresh HTTP request. + """ keys_to_delete = [k for k in self.cache if k[0] == query] for k in keys_to_delete: del self.cache[k] diff --git a/src/babel_validation/services/nodenorm.py b/src/babel_validation/services/nodenorm.py index 633f7a4b..5f858c4f 100644 --- a/src/babel_validation/services/nodenorm.py +++ b/src/babel_validation/services/nodenorm.py @@ -1,3 +1,21 @@ +""" +Cached client for the NodeNorm ``get_normalized_nodes`` API. + +Caching model +------------- +Each response is stored under the key ``(curie, frozenset(params.items()))``. +Entries are never evicted automatically; call ``clear_curie()`` to force +a fresh lookup for a specific identifier. + +Cache-warming pattern +--------------------- +When you need to normalize many CURIEs for the same logical task (e.g. all +CURIEs referenced in a GitHub issue), call ``normalize_curies()`` once with +the full list. That issues a single HTTP request and populates the cache. +Subsequent ``normalize_curie()`` calls for any of those identifiers return +immediately from cache — no additional HTTP traffic. +""" + import logging import time @@ -16,11 +34,30 @@ def __str__(self): @staticmethod def from_url(nodenorm_url: str) -> 'CachedNodeNorm': + """Return the singleton ``CachedNodeNorm`` for *nodenorm_url*. + + The singleton ensures that cache entries accumulated during one part of + a test run are reused by later parts that share the same URL. Prefer + this over direct construction unless you explicitly want a fresh cache. + """ if nodenorm_url not in cached_node_norms_by_url: cached_node_norms_by_url[nodenorm_url] = CachedNodeNorm(nodenorm_url) return cached_node_norms_by_url[nodenorm_url] def normalize_curies(self, curies: list[str], **params) -> dict[str, dict]: + """Normalize *curies* in bulk, returning a ``{curie: result}`` mapping. + + Already-cached CURIEs are served from the cache; the remainder are + fetched from NodeNorm in a single HTTP POST to ``get_normalized_nodes``. + The response is merged with the cached results before returning. + + *curies* must be a non-empty list — the NodeNorm API rejects empty + requests, so this method raises ``ValueError`` immediately. + + Values in the returned dict are ``None`` for CURIEs NodeNorm could not + resolve. Use this as the cache-warming call; subsequent + ``normalize_curie()`` calls for these identifiers will be free. + """ if not curies: raise ValueError(f"curies must not be empty when calling normalize_curies({curies}, {params}) on {self}") if not isinstance(curies, list): @@ -56,14 +93,28 @@ def normalize_curies(self, curies: list[str], **params) -> dict[str, dict]: return result def normalize_curie(self, curie, **params): + """Normalize a single *curie*, returning the NodeNorm result or ``None``. + + Checks the cache first; on a miss, delegates to ``normalize_curies()`` + (one HTTP call) and returns the result. If you expect to normalize many + CURIEs, call ``normalize_curies()`` upfront so this method never makes + an HTTP call. + + Uses ``.get()`` rather than direct indexing so that a NodeNorm response + that silently omits a requested CURIE returns ``None`` instead of + raising ``KeyError``. + """ cache_key = (curie, frozenset(params.items())) if cache_key in self.cache: return self.cache[cache_key] - # Use .get(): NodeNorm normally echoes every requested CURIE (null when - # unresolvable), but don't crash if it ever omits one. return self.normalize_curies([curie], **params).get(curie) def clear_curie(self, curie): + """Remove all cached results for *curie* (across every param variant). + + The next call to ``normalize_curie()`` or ``normalize_curies()`` for + this identifier will issue a fresh HTTP request. + """ keys_to_delete = [k for k in self.cache if k[0] == curie] for k in keys_to_delete: del self.cache[k] From 362f4eb02867706464aa2a881474f8d6f5487217 Mon Sep 17 00:00:00 2001 From: Gaurav Vaidya Date: Fri, 26 Jun 2026 14:23:50 -0400 Subject: [PATCH 09/23] Add Protocol interfaces and rename cache-invalidation methods MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit NodeNormService and NameResService Protocols document the public interface callers should type against. When the implementation is later replaced by an external library, any code typed against the Protocol will need no changes. Rename clear_curie() → invalidate_curie() and delete_query() → invalidate_query(). "Invalidate" is standard cache vocabulary and makes the full-eviction-across-all-param-variants semantics clearer than "clear/delete". Also tightens type annotations on normalize_curie / lookup signatures (str, return type) to match the Protocol. Co-Authored-By: Claude Sonnet 4.6 --- src/babel_validation/services/nameres.py | 21 ++++++++++++++++++--- src/babel_validation/services/nodenorm.py | 23 +++++++++++++++++++---- 2 files changed, 37 insertions(+), 7 deletions(-) diff --git a/src/babel_validation/services/nameres.py b/src/babel_validation/services/nameres.py index aef6ac2e..6aadd85f 100644 --- a/src/babel_validation/services/nameres.py +++ b/src/babel_validation/services/nameres.py @@ -4,7 +4,7 @@ Caching model ------------- Each response is stored under the key ``(query, frozenset(params.items()))``. -Entries are never evicted automatically; call ``delete_query()`` to force +Entries are never evicted automatically; call ``invalidate_query()`` to force a fresh lookup for a specific query string. Cache-warming pattern @@ -24,11 +24,26 @@ import logging import time +from typing import Protocol import requests cached_nameres_by_url = {} + +class NameResService(Protocol): + """Interface that callers should depend on. + + Type parameters against this Protocol rather than ``CachedNameRes`` + directly so that a future drop-in library replacement requires no caller + changes. + """ + + def bulk_lookup(self, queries: list[str], **params) -> dict[str, dict]: ... + def lookup(self, query: str, **params) -> list[dict]: ... + def invalidate_query(self, query: str) -> None: ... + + class CachedNameRes: def __init__(self, nameres_url: str): self.nameres_url = nameres_url @@ -96,7 +111,7 @@ def bulk_lookup(self, queries: list[str], **params) -> dict[str, dict]: return result - def lookup(self, query, **params): + def lookup(self, query: str, **params) -> list[dict]: """Look up a single *query* string via the NameRes ``/lookup`` endpoint. This targets a different endpoint from ``bulk_lookup()`` — parameters @@ -122,7 +137,7 @@ def lookup(self, query, **params): self.cache[cache_key] = result return result - def delete_query(self, query): + def invalidate_query(self, query: str) -> None: """Remove all cached results for *query* (across every param variant). The next call to ``lookup()`` or ``bulk_lookup()`` for this query will diff --git a/src/babel_validation/services/nodenorm.py b/src/babel_validation/services/nodenorm.py index 5f858c4f..9ba3242b 100644 --- a/src/babel_validation/services/nodenorm.py +++ b/src/babel_validation/services/nodenorm.py @@ -4,7 +4,7 @@ Caching model ------------- Each response is stored under the key ``(curie, frozenset(params.items()))``. -Entries are never evicted automatically; call ``clear_curie()`` to force +Entries are never evicted automatically; call ``invalidate_curie()`` to force a fresh lookup for a specific identifier. Cache-warming pattern @@ -18,11 +18,26 @@ import logging import time +from typing import Protocol import requests cached_node_norms_by_url = {} + +class NodeNormService(Protocol): + """Interface that callers should depend on. + + Type parameters against this Protocol rather than ``CachedNodeNorm`` + directly so that a future drop-in library replacement requires no caller + changes. + """ + + def normalize_curies(self, curies: list[str], **params) -> dict[str, dict | None]: ... + def normalize_curie(self, curie: str, **params) -> dict | None: ... + def invalidate_curie(self, curie: str) -> None: ... + + class CachedNodeNorm: def __init__(self, nodenorm_url: str): self.nodenorm_url = nodenorm_url @@ -44,7 +59,7 @@ def from_url(nodenorm_url: str) -> 'CachedNodeNorm': cached_node_norms_by_url[nodenorm_url] = CachedNodeNorm(nodenorm_url) return cached_node_norms_by_url[nodenorm_url] - def normalize_curies(self, curies: list[str], **params) -> dict[str, dict]: + def normalize_curies(self, curies: list[str], **params) -> dict[str, dict | None]: """Normalize *curies* in bulk, returning a ``{curie: result}`` mapping. Already-cached CURIEs are served from the cache; the remainder are @@ -92,7 +107,7 @@ def normalize_curies(self, curies: list[str], **params) -> dict[str, dict]: return result - def normalize_curie(self, curie, **params): + def normalize_curie(self, curie: str, **params) -> dict | None: """Normalize a single *curie*, returning the NodeNorm result or ``None``. Checks the cache first; on a miss, delegates to ``normalize_curies()`` @@ -109,7 +124,7 @@ def normalize_curie(self, curie, **params): return self.cache[cache_key] return self.normalize_curies([curie], **params).get(curie) - def clear_curie(self, curie): + def invalidate_curie(self, curie: str) -> None: """Remove all cached results for *curie* (across every param variant). The next call to ``normalize_curie()`` or ``normalize_curies()`` for From 35b52a5b2d16f2f1d99647c27c4c38c7f28ed329 Mon Sep 17 00:00:00 2001 From: Gaurav Vaidya Date: Fri, 26 Jun 2026 01:13:40 -0400 Subject: [PATCH 10/23] Add standard Python .gitignore entries The repo's .gitignore predates the Python rewrite (it only covered Scala/Giter8/IntelliJ artifacts), so __pycache__/ and *.pyc were tracked as untracked noise and easy to commit by accident. Append the standard GitHub Python.gitignore template (bytecode, build/dist, .pytest_cache, .venv/.env, mypy/coverage caches, etc.) plus a .DS_Store entry for macOS. Co-Authored-By: Claude Opus 4.8 --- .gitignore | 152 +++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 152 insertions(+) diff --git a/.gitignore b/.gitignore index a1480ec8..34bc7dfb 100644 --- a/.gitignore +++ b/.gitignore @@ -11,3 +11,155 @@ target/ # Ignore .idea, files specific to the IntelliJ IDE. .idea/ + +# macOS +.DS_Store + +# --- Python (GitHub Python.gitignore template) --- + +# Byte-compiled / optimized / DLL files +__pycache__/ +*.py[cod] +*$py.class + +# C extensions +*.so + +# Distribution / packaging +.Python +build/ +develop-eggs/ +dist/ +downloads/ +eggs/ +.eggs/ +lib/ +lib64/ +parts/ +sdist/ +var/ +wheels/ +share/python-wheels/ +*.egg-info/ +.installed.cfg +*.egg +MANIFEST + +# PyInstaller +# Usually these files are written by a python script from a template +# before PyInstaller builds the exe, so as to inject date/other infos into it. +*.manifest +*.spec + +# Installer logs +pip-log.txt +pip-delete-this-directory.txt + +# Unit test / coverage reports +htmlcov/ +.tox/ +.nox/ +.coverage +.coverage.* +.cache +nosetests.xml +coverage.xml +*.cover +*.py,cover +.hypothesis/ +.pytest_cache/ +cover/ + +# Translations +*.mo +*.pot + +# Django stuff: +local_settings.py +db.sqlite3 +db.sqlite3-journal + +# Flask stuff: +instance/ +.webassets-cache + +# Scrapy stuff: +.scrapy + +# Sphinx documentation +docs/_build/ + +# PyBuilder +.pybuilder/ + +# Jupyter Notebook +.ipynb_checkpoints + +# IPython +profile_default/ +ipython_config.py + +# pyenv +# For a library or package, you might want to ignore these files since the code is +# intended to run in multiple environments; otherwise, check them in: +# .python-version + +# pipenv +# According to pypa/pipenv#598, it is recommended to include Pipfile.lock in version control. +#Pipfile.lock + +# UV +# This project keeps uv.lock in version control (see pyproject.toml). +#uv.lock + +# poetry +#poetry.lock + +# pdm +#pdm.lock +.pdm.toml +.pdm-python +.pdm-build/ + +# PEP 582; used by e.g. github.com/David-OConnor/pyflow and github.com/pdm-project/pdm +__pypackages__/ + +# Celery stuff +celerybeat-schedule +celerybeat.pid + +# SageMath parsed files +*.sage.py + +# Environments +.env +.venv +env/ +venv/ +ENV/ +env.bak/ +venv.bak/ + +# Spyder project settings +.spyderproject +.spyproject + +# Rope project settings +.ropeproject + +# mkdocs documentation +/site + +# mypy +.mypy_cache/ +.dmypy.json +dmypy.json + +# Pyre type checker +.pyre/ + +# pytype static type analyzer +.pytype/ + +# Cython debug symbols +cython_debug/ From 1d6030e38c5a3425b1564a8040b117c84fa0bb26 Mon Sep 17 00:00:00 2001 From: Gaurav Vaidya Date: Fri, 26 Jun 2026 01:19:01 -0400 Subject: [PATCH 11/23] Apply suggestion from @gaurav --- .gitignore | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.gitignore b/.gitignore index 34bc7dfb..97083602 100644 --- a/.gitignore +++ b/.gitignore @@ -15,7 +15,7 @@ target/ # macOS .DS_Store -# --- Python (GitHub Python.gitignore template) --- +# --- Python (GitHub Python.gitignore template from https://github.com/github/gitignore/blob/main/Python.gitignore) --- # Byte-compiled / optimized / DLL files __pycache__/ From be551588afa14382ea227331d5c0557c2a46f358 Mon Sep 17 00:00:00 2001 From: Gaurav Vaidya Date: Tue, 18 Aug 2026 16:26:28 -0400 Subject: [PATCH 12/23] Guarantee normalize_curies() returns an entry per requested CURIE MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit normalize_curies() built its return value from response.json(), so a CURIE that NodeNorm silently omitted from its response was simply absent from the returned dict rather than present with a None value. A caller iterating results.items() would never see it, and would report success for a CURIE it never actually tested. The warm-cache path happened to add the missing key back, so the hole only opened on a cold cache — which is exactly when a first-time lookup happens. Build the result from *curies* instead. Every requested CURIE is in the cache by that point, so the dict now has exactly one entry per request, in request order. Request ordering also makes downstream "first result that resolved" logic deterministic; previously it depended on the server's JSON ordering on a cold cache and on set iteration order (i.e. per-process string hash randomization) on a warm one. Co-Authored-By: Claude Opus 5 --- src/babel_validation/services/nodenorm.py | 19 +++++++++++-------- 1 file changed, 11 insertions(+), 8 deletions(-) diff --git a/src/babel_validation/services/nodenorm.py b/src/babel_validation/services/nodenorm.py index 9ba3242b..55cc224a 100644 --- a/src/babel_validation/services/nodenorm.py +++ b/src/babel_validation/services/nodenorm.py @@ -69,9 +69,12 @@ def normalize_curies(self, curies: list[str], **params) -> dict[str, dict | None *curies* must be a non-empty list — the NodeNorm API rejects empty requests, so this method raises ``ValueError`` immediately. - Values in the returned dict are ``None`` for CURIEs NodeNorm could not - resolve. Use this as the cache-warming call; subsequent - ``normalize_curie()`` calls for these identifiers will be free. + The returned dict has exactly one entry per requested CURIE, in the + order requested, with a value of ``None`` for CURIEs NodeNorm could not + resolve or silently omitted from its response. Callers may therefore + iterate it and trust that every CURIE they asked about is represented. + Use this as the cache-warming call; subsequent ``normalize_curie()`` + calls for these identifiers will be free. """ if not curies: raise ValueError(f"curies must not be empty when calling normalize_curies({curies}, {params}) on {self}") @@ -85,7 +88,6 @@ def normalize_curies(self, curies: list[str], **params) -> dict[str, dict | None curies_to_be_queried = curies_set - cached_curies # Make query. - result = {} if curies_to_be_queried: api_params = dict(params) api_params['curies'] = list(curies_to_be_queried) @@ -98,14 +100,15 @@ def normalize_curies(self, curies: list[str], **params) -> dict[str, dict | None for curie in curies_to_be_queried: self.cache[(curie, params_key)] = result.get(curie, None) - for curie in cached_curies: - result[curie] = self.cache[(curie, params_key)] - time_taken_sec = (time.time_ns() - time_started) / 1E9 self.logger.info("Normalizing %d CURIEs %s (with %d CURIEs cached) with params %s on %s in %.3fs", len(curies_to_be_queried), curies_to_be_queried, len(cached_curies), params, self, time_taken_sec) - return result + # Build the result from *curies*, not from the response: NodeNorm may + # silently omit a requested CURIE, and a missing key is invisible to a + # caller that iterates the returned dict. Every CURIE is in the cache by + # this point, either from a previous call or from the loop above. + return {curie: self.cache[(curie, params_key)] for curie in curies} def normalize_curie(self, curie: str, **params) -> dict | None: """Normalize a single *curie*, returning the NodeNorm result or ``None``. From 99de9df25187479d3354f4f1e7b22f784e1e2f47 Mon Sep 17 00:00:00 2001 From: Gaurav Vaidya Date: Tue, 18 Aug 2026 16:26:28 -0400 Subject: [PATCH 13/23] Strip params and validate CURIEs on the NameRes path too MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CURIE-format validation and NodeNorm cache warming lived in NodeNormTest.test_with_nodenorm(), so NameRes assertions got neither: SearchByName's expected CURIE went to NodeNorm unvalidated, contradicting the documented "malformed CURIEs are never sent to NodeNorm" invariant, and each param_set cost its own NodeNorm round-trip instead of one batched call. Hoist the logic to AssertionHandler.prepare_param_sets() and call it from both test_with_nodenorm() and test_with_nameres(). SearchByName overrides curie_params() to params[1:2] — its first param is a free-text search query. The slice rather than an index keeps a malformed param_set out of validation so test_param_set() reports the arity problem instead. prepare_param_sets() also strips surrounding whitespace from every param. _CURIE_RE is anchored, so a wiki-syntax param with incidental padding ({{BabelTest|Resolves| CHEBI:15365 }}) was reported as malformed; HasLabel already stripped its label param, so this makes the treatment uniform. Adds a VALIDATE_CURIES class attribute (default True) for assertions that need to opt out. Co-Authored-By: Claude Opus 5 --- src/babel_validation/assertions/__init__.py | 85 +++++++++++++-------- src/babel_validation/assertions/nameres.py | 6 ++ 2 files changed, 59 insertions(+), 32 deletions(-) diff --git a/src/babel_validation/assertions/__init__.py b/src/babel_validation/assertions/__init__.py index ae82b4c1..6d122fba 100644 --- a/src/babel_validation/assertions/__init__.py +++ b/src/babel_validation/assertions/__init__.py @@ -30,12 +30,61 @@ class AssertionHandler: NAME: str # lowercase assertion name as used in issue bodies DESCRIPTION: str # one-line human-readable description + # Whether CURIE params should be rejected up front if they are not well-formed. + # Assertions about deliberately-invalid identifiers turn this off. + VALIDATE_CURIES = True + + _CURIE_RE = re.compile(r'^[A-Za-z][A-Za-z0-9._-]*:[^\s]+$') + def passed(self, message: str) -> TestResult: return TestResult(status=TestStatus.Passed, message=message) def failed(self, message: str) -> TestResult: return TestResult(status=TestStatus.Failed, message=message) + def curie_params(self, params: list[str]) -> list[str]: + """Return the subset of params that are CURIEs (for prewarming and validation). + Default: all params are CURIEs. Subclasses override when some params are non-CURIEs.""" + return params + + def prepare_param_sets(self, param_sets: list[list[str]], nodenorm, + label: str = "") -> tuple[list[list[str]], dict[int, TestResult]]: + """Strip params, reject unusable param_sets, and warm the NodeNorm cache. + + Returns ``(stripped_param_sets, failures)``, where *failures* maps a + param_set index to the TestResult explaining why it was rejected. + Rejected param_sets are excluded from cache warming, so (unless + VALIDATE_CURIES is off) malformed CURIEs are never sent to NodeNorm. + """ + stripped = [[param.strip() for param in params] for params in param_sets] + + failures: dict[int, TestResult] = {} + for index, params in enumerate(stripped): + if not params: + failures[index] = self.failed(f"No parameters in param_set {index} in {label}") + continue + if not self.VALIDATE_CURIES: + continue + invalid = [c for c in self.curie_params(params) if not self._CURIE_RE.match(c)] + if invalid: + failures[index] = self.failed( + f"Malformed CURIE(s) {invalid} in param_set {index} in {label}: " + f"expected format PREFIX:LOCAL_ID (e.g. CHEBI:15365)" + ) + + # Warm the cache in a single request, deduplicated; skip if empty + # (normalize_curies raises ValueError on an empty list). + curies_to_warm = list({ + p + for index, params in enumerate(stripped) + if index not in failures + for p in self.curie_params(params) + }) + if curies_to_warm: + nodenorm.normalize_curies(curies_to_warm) + + return stripped, failures + def test_with_nodenorm(self, param_sets: list[list[str]], nodenorm, label: str = "") -> Iterator[TestResult]: """Evaluate this assertion against NodeNorm. Returns nothing if not applicable.""" @@ -54,41 +103,12 @@ class NodeNormTest(AssertionHandler): Subclasses implement test_param_set() instead of test_with_nodenorm(). """ - _CURIE_RE = re.compile(r'^[A-Za-z][A-Za-z0-9._-]*:[^\s]+$') - - def curie_params(self, params: list[str]) -> list[str]: - """Return the subset of params that are CURIEs (for prewarming and validation). - Default: all params are CURIEs. Subclasses override when some params are non-CURIEs.""" - return params - def test_with_nodenorm(self, param_sets: list[list[str]], nodenorm, label: str = "") -> Iterator[TestResult]: if not param_sets: yield self.failed(f"No parameters provided in {label}") return - # Validate each param_set up front so malformed CURIEs are never sent to - # NodeNorm — not even in the cache-warming call below. - failures: dict[int, TestResult] = {} - for index, params in enumerate(param_sets): - if not params: - failures[index] = self.failed(f"No parameters in param_set {index} in {label}") - continue - invalid = [c for c in self.curie_params(params) if not self._CURIE_RE.match(c)] - if invalid: - failures[index] = self.failed( - f"Malformed CURIE(s) {invalid} in param_set {index} in {label}: " - f"expected format PREFIX:LOCAL_ID (e.g. CHEBI:15365)" - ) - # warm the cache only for params that are CURIEs (deduplicated); skip if empty - # (normalize_curies raises ValueError on an empty list) - curies_to_warm = list({ - p - for index, params in enumerate(param_sets) - if index not in failures - for p in self.curie_params(params) - }) - if curies_to_warm: - nodenorm.normalize_curies(curies_to_warm) + param_sets, failures = self.prepare_param_sets(param_sets, nodenorm, label) results = [] for index, params in enumerate(param_sets): if index in failures: @@ -132,10 +152,11 @@ def test_with_nameres(self, param_sets: list[list[str]], nodenorm, nameres, if not param_sets: yield self.failed(f"No parameters provided in {label}") return + param_sets, failures = self.prepare_param_sets(param_sets, nodenorm, label) results = [] for index, params in enumerate(param_sets): - if not params: - results.append(self.failed(f"No parameters in param_set {index} in {label}")) + if index in failures: + results.append(failures[index]) continue results.extend(self.test_param_set(params, nodenorm, nameres, pass_if_found_in_top, label)) if not results: diff --git a/src/babel_validation/assertions/nameres.py b/src/babel_validation/assertions/nameres.py index a0b493f0..b406a54a 100644 --- a/src/babel_validation/assertions/nameres.py +++ b/src/babel_validation/assertions/nameres.py @@ -23,6 +23,12 @@ class SearchByNameHandler(NameResTest): WIKI_EXAMPLES = ["{{BabelTest|SearchByName|water|CHEBI:15377}}"] YAML_PARAMS = " - [water, CHEBI:15377]\n - [diabetes, MONDO:0005015]" + def curie_params(self, params: list[str]) -> list[str]: + # params[0] is a free-text search query; only the expected CURIE is a CURIE. + # Slicing (rather than indexing) keeps malformed param_sets out of validation + # so test_param_set() can report the arity problem instead. + return params[1:2] + def test_param_set(self, params: list[str], nodenorm: CachedNodeNorm, nameres: CachedNameRes, pass_if_found_in_top: int = 5, label: str = "") -> Iterator[TestResult]: From 8d9e6b054284311f92e114804454d0021b2f32b4 Mon Sep 17 00:00:00 2001 From: Gaurav Vaidya Date: Tue, 18 Aug 2026 16:26:28 -0400 Subject: [PATCH 14/23] Let DoesNotResolve assert about malformed identifiers Up-front CURIE-format validation applied to DoesNotResolve, which is the one assertion whose entire purpose is identifiers that don't resolve. {{BabelTest|DoesNotResolve|not a curie}} failed with "Malformed CURIE(s)" rather than passing, so the assertion could not express "this junk identifier must not resolve". Set VALIDATE_CURIES = False on the handler. A param that isn't a well-formed CURIE trivially does not resolve, which is the assertion's expected outcome. Co-Authored-By: Claude Opus 5 --- src/babel_validation/assertions/nodenorm.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/babel_validation/assertions/nodenorm.py b/src/babel_validation/assertions/nodenorm.py index 92f477f5..1451e7f2 100644 --- a/src/babel_validation/assertions/nodenorm.py +++ b/src/babel_validation/assertions/nodenorm.py @@ -37,6 +37,10 @@ class DoesNotResolveHandler(NodeNormTest): WIKI_EXAMPLES = ["{{BabelTest|DoesNotResolve|FAKENS:99999}}"] YAML_PARAMS = " - FAKENS:99999" + # A param that isn't even a well-formed CURIE trivially does not resolve, and + # asserting that is the whole point of this assertion — so don't reject it. + VALIDATE_CURIES = False + def test_param_set(self, params: list[str], nodenorm: CachedNodeNorm, label: str = "") -> Iterator[TestResult]: for curie in params: @@ -55,6 +59,8 @@ def _compare_resolutions( first_good_result is None if every CURIE failed to resolve. per_curie_results maps each CURIE to its result (None if unresolvable). """ + # normalize_curies() guarantees one entry per requested CURIE, in the order + # requested, so first_good is deterministically the first param that resolved. per_curie = nodenorm.normalize_curies(params) first_good = next((r for r in per_curie.values() if r is not None), None) return first_good, per_curie From 84cbd7258888d33341e2b9444cb132b91cf43ede Mon Sep 17 00:00:00 2001 From: Gaurav Vaidya Date: Tue, 18 Aug 2026 16:26:28 -0400 Subject: [PATCH 15/23] Group README assertions by service rather than registration order generate_readme() emitted a group header the first time it saw a handler for that service while walking ASSERTION_HANDLERS in insertion order. The registry happens to be grouped today, so the output is correct; register a NodeNormTest after SearchByNameHandler and it renders under "## NameRes Assertions" with no header of its own. test_assertions_docs.py can't catch it either, since it regenerates the same wrong output. Iterate _GROUP_HEADERS and filter handlers per group instead. The README regenerates byte-identical. Co-Authored-By: Claude Opus 5 --- src/babel_validation/assertions/gen_docs.py | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/babel_validation/assertions/gen_docs.py b/src/babel_validation/assertions/gen_docs.py index c5a77a43..36808855 100644 --- a/src/babel_validation/assertions/gen_docs.py +++ b/src/babel_validation/assertions/gen_docs.py @@ -127,14 +127,15 @@ def _render_handler(h: AssertionHandler) -> str: def generate_readme() -> str: sections = [INTRO] - seen_groups: set[str] = set() - - for h in ASSERTION_HANDLERS.values(): - service = _applies_to(h) - if service not in seen_groups: - seen_groups.add(service) - sections.append(_GROUP_HEADERS[service] + "\n") - sections.append(_render_handler(h)) + + # Group by service rather than by registration order, so a handler added + # anywhere in ASSERTION_HANDLERS still renders under the right heading. + for service, header in _GROUP_HEADERS.items(): + handlers = [h for h in ASSERTION_HANDLERS.values() if _applies_to(h) == service] + if not handlers: + continue + sections.append(header + "\n") + sections.extend(_render_handler(h) for h in handlers) sections.append(ADDING_NEW) return "\n".join(sections) From cd48090afb66882e89ac04868ae406ea63ea9e8c Mon Sep 17 00:00:00 2001 From: Gaurav Vaidya Date: Tue, 18 Aug 2026 16:26:28 -0400 Subject: [PATCH 16/23] Add offline unit tests for the assertion handlers Stubs requests.post rather than the service, so the tests run against the real CachedNodeNorm and exercise its bulk-normalization contract instead of restating it in a fake. The fixture DB drops one CURIE from the response entirely, which is what NodeNorm does for some unknown identifiers. Covers the omitted-CURIE contract, ResolvesWith/DoesNotResolveWith handling of an unresolvable CURIE, which CURIE gets blamed when one of three differs, DoesNotResolve accepting a malformed identifier, param stripping, SearchByName rejecting a bad CURIE without calling NodeNorm, and README grouping for a handler registered out of order. Co-Authored-By: Claude Opus 5 --- tests/test_environment/test_assertions.py | 131 ++++++++++++++++++++++ 1 file changed, 131 insertions(+) create mode 100644 tests/test_environment/test_assertions.py diff --git a/tests/test_environment/test_assertions.py b/tests/test_environment/test_assertions.py new file mode 100644 index 00000000..704e0a5b --- /dev/null +++ b/tests/test_environment/test_assertions.py @@ -0,0 +1,131 @@ +"""Unit tests for the assertion handlers, with NodeNorm's HTTP layer stubbed out. + +These run against the real CachedNodeNorm so that the bulk-normalization contract +(one entry per requested CURIE) is exercised, not just re-stated by a fake. +""" + +import pytest + +from src.babel_validation.assertions import ASSERTION_HANDLERS, NodeNormTest +from src.babel_validation.assertions.gen_docs import generate_readme +from src.babel_validation.assertions.nodenorm import ( + DoesNotResolveHandler, DoesNotResolveWithHandler, ResolvesHandler, ResolvesWithHandler, +) +from src.babel_validation.core.testrow import TestStatus +from src.babel_validation.services import nodenorm as nodenorm_service + + +def _node(identifier, label): + return {'id': {'identifier': identifier, 'label': label}, 'type': ['biolink:SmallMolecule']} + + +# A:1 and B:1 are equivalent; C:1 is a distinct entity; D:1 is dropped from the +# response entirely, which is what NodeNorm does for some unknown identifiers. +FAKE_NODENORM_DB = { + 'A:1': _node('A:1', 'alpha'), + 'B:1': _node('A:1', 'alpha'), + 'C:1': _node('C:1', 'gamma'), +} + + +@pytest.fixture +def nodenorm(monkeypatch): + """A CachedNodeNorm backed by FAKE_NODENORM_DB, with a .post_count attribute.""" + calls = [] + + class FakeResponse: + def __init__(self, payload): + self._payload = payload + + def raise_for_status(self): + pass + + def json(self): + return self._payload + + def fake_post(url, json=None, timeout=None): + calls.append(json['curies']) + return FakeResponse({c: FAKE_NODENORM_DB[c] for c in json['curies'] if c in FAKE_NODENORM_DB}) + + monkeypatch.setattr(nodenorm_service.requests, 'post', fake_post) + service = nodenorm_service.CachedNodeNorm('http://fake-nodenorm.example/') + service.post_calls = calls + return service + + +def _messages(results): + return [(r.status, r.message) for r in results] + + +@pytest.mark.unit +def test_normalize_curies_covers_every_requested_curie(nodenorm): + """NodeNorm omitting a CURIE must surface as None, not as a missing key.""" + results = nodenorm.normalize_curies(['A:1', 'D:1']) + assert list(results) == ['A:1', 'D:1'] + assert results['D:1'] is None + + +@pytest.mark.unit +def test_resolves_with_fails_on_omitted_curie(nodenorm): + results = list(ResolvesWithHandler().test_with_nodenorm([['A:1', 'D:1']], nodenorm, 'test')) + failures = [m for status, m in _messages(results) if status == TestStatus.Failed] + assert any('D:1' in m for m in failures), _messages(results) + + +@pytest.mark.unit +def test_does_not_resolve_with_fails_on_omitted_curie(nodenorm): + """The 'every CURIE must resolve' guard must see the dropped CURIE.""" + results = list(DoesNotResolveWithHandler().test_with_nodenorm([['A:1', 'C:1', 'D:1']], nodenorm, 'test')) + assert all(status == TestStatus.Failed for status, _ in _messages(results)), _messages(results) + assert any('D:1' in m for _, m in _messages(results)) + + +@pytest.mark.unit +def test_resolves_with_blames_the_odd_curie_out(nodenorm): + """The canonical identifier comes from the first param, so C:1 is the failure.""" + results = list(ResolvesWithHandler().test_with_nodenorm([['A:1', 'B:1', 'C:1']], nodenorm, 'test')) + failures = [m for status, m in _messages(results) if status == TestStatus.Failed] + assert len(failures) == 1 and failures[0].startswith('Resolved C:1'), _messages(results) + + +@pytest.mark.unit +def test_does_not_resolve_accepts_a_malformed_identifier(nodenorm): + """A junk identifier is exactly what DoesNotResolve exists to assert about.""" + results = list(DoesNotResolveHandler().test_with_nodenorm([['not a curie']], nodenorm, 'test')) + assert [status for status, _ in _messages(results)] == [TestStatus.Passed], _messages(results) + + +@pytest.mark.unit +def test_surrounding_whitespace_is_stripped(nodenorm): + results = list(ResolvesHandler().test_with_nodenorm([[' A:1 ']], nodenorm, 'test')) + assert [status for status, _ in _messages(results)] == [TestStatus.Passed], _messages(results) + + +@pytest.mark.unit +def test_search_by_name_validates_and_warms_before_calling_nodenorm(nodenorm): + """The NameRes path gets the same CURIE validation as the NodeNorm path.""" + handler = ASSERTION_HANDLERS['searchbyname'] + results = list(handler.test_with_nameres([['water', 'not a curie']], nodenorm, None, 5, 'test')) + assert [status for status, _ in _messages(results)] == [TestStatus.Failed], _messages(results) + assert nodenorm.post_calls == [] + + +class TempGroupingHandler(NodeNormTest): + """Registered last, after the NameRes handlers, only by test_docs_group_handlers_*.""" + NAME = 'tempgrouping' + DESCRIPTION = 'Temporary handler used to check README grouping.' + PARAMETERS = '' + WIKI_EXAMPLES = ['{{BabelTest|TempGrouping|A:1}}'] + YAML_PARAMS = ' - A:1' + + +@pytest.mark.unit +def test_docs_group_handlers_by_service_not_registration_order(): + """A NodeNorm handler registered last must still render under NodeNorm.""" + ASSERTION_HANDLERS[TempGroupingHandler.NAME] = TempGroupingHandler() + try: + readme = generate_readme() + finally: + del ASSERTION_HANDLERS[TempGroupingHandler.NAME] + assert '### TempGrouping' in readme + assert readme.index('### TempGrouping') < readme.index('## NameRes Assertions') From da1fa408935e3867a4007f29647f5942bb383b09 Mon Sep 17 00:00:00 2001 From: Gaurav Vaidya Date: Tue, 18 Aug 2026 16:33:46 -0400 Subject: [PATCH 17/23] Correct stale normalize_curies() docstring line "The response is merged with the cached results before returning" described the old implementation, which built its return value from response.json() and then added the cached entries to it. The method now caches the response and assembles the return value from the cache, which is what lets it guarantee an entry per requested CURIE. Co-Authored-By: Claude Opus 5 --- src/babel_validation/services/nodenorm.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/babel_validation/services/nodenorm.py b/src/babel_validation/services/nodenorm.py index 55cc224a..abf4006b 100644 --- a/src/babel_validation/services/nodenorm.py +++ b/src/babel_validation/services/nodenorm.py @@ -63,8 +63,8 @@ def normalize_curies(self, curies: list[str], **params) -> dict[str, dict | None """Normalize *curies* in bulk, returning a ``{curie: result}`` mapping. Already-cached CURIEs are served from the cache; the remainder are - fetched from NodeNorm in a single HTTP POST to ``get_normalized_nodes``. - The response is merged with the cached results before returning. + fetched from NodeNorm in a single HTTP POST to ``get_normalized_nodes`` + and cached. The return value is then assembled from the cache. *curies* must be a non-empty list — the NodeNorm API rejects empty requests, so this method raises ``ValueError`` immediately. From 02d7ddf43096e4fb85efa73df3892809adfdb630 Mon Sep 17 00:00:00 2001 From: Gaurav Vaidya Date: Wed, 19 Aug 2026 01:39:45 -0400 Subject: [PATCH 18/23] Name the param-set types instead of nesting list[str] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signatures like `tuple[list[list[str]], dict[int, TestResult]]` said nothing about what the strings were or how the two halves related. Introduce Params (one assertion invocation's parameters) and ParamSets (a list of them) as aliases, and use them throughout the handler modules. The deeper problem was that prepare_param_sets() returned two structures keyed by the same index — the stripped param_sets and a dict of failures — leaving both NodeNormTest and NameResTest to re-zip them by hand with an index lookup. Return a list of PreparedParamSet instead, a frozen two-field record pairing each param_set with the reason it was rejected (None when it wasn't). The callers become a single loop over prepared param_sets with no index bookkeeping. Also splits the per-param_set rejection check out into _rejection(), so prepare_param_sets() reads as strip / reject / warm rather than interleaving all three. No behaviour change; the generated README is byte-identical. Co-Authored-By: Claude Opus 5 --- src/babel_validation/assertions/__init__.py | 107 ++++++++++++-------- src/babel_validation/assertions/nameres.py | 6 +- src/babel_validation/assertions/nodenorm.py | 20 ++-- 3 files changed, 78 insertions(+), 55 deletions(-) diff --git a/src/babel_validation/assertions/__init__.py b/src/babel_validation/assertions/__init__.py index 6d122fba..e945f3ea 100644 --- a/src/babel_validation/assertions/__init__.py +++ b/src/babel_validation/assertions/__init__.py @@ -20,10 +20,32 @@ """ import re +from dataclasses import dataclass from typing import Iterator from src.babel_validation.core.testrow import TestResult, TestStatus +# The parameters of a single assertion invocation, e.g. ["CHEBI:15365", "aspirin"] +# for {{BabelTest|HasLabel|CHEBI:15365|aspirin}}. What each element means depends +# on the assertion; see the handler's PARAMETERS attribute. +Params = list[str] + +# The param_sets of one assertion: each is evaluated independently, and each +# produces its own TestResults. +ParamSets = list[Params] + + +@dataclass(frozen=True) +class PreparedParamSet: + """One param_set after stripping and validation, ready to be evaluated. + + *failure* is None when the param_set is usable. When it is set, the + param_set was rejected before reaching the service and *failure* is the + TestResult to report in its place. + """ + params: Params + failure: TestResult | None = None + class AssertionHandler: """Base class for all BabelTest assertion handlers.""" @@ -42,55 +64,57 @@ def passed(self, message: str) -> TestResult: def failed(self, message: str) -> TestResult: return TestResult(status=TestStatus.Failed, message=message) - def curie_params(self, params: list[str]) -> list[str]: + def curie_params(self, params: Params) -> Params: """Return the subset of params that are CURIEs (for prewarming and validation). Default: all params are CURIEs. Subclasses override when some params are non-CURIEs.""" return params - def prepare_param_sets(self, param_sets: list[list[str]], nodenorm, - label: str = "") -> tuple[list[list[str]], dict[int, TestResult]]: + def prepare_param_sets(self, param_sets: ParamSets, nodenorm, + label: str = "") -> list[PreparedParamSet]: """Strip params, reject unusable param_sets, and warm the NodeNorm cache. - Returns ``(stripped_param_sets, failures)``, where *failures* maps a - param_set index to the TestResult explaining why it was rejected. + Returns one PreparedParamSet per input param_set, in order, each either + carrying stripped params or a failure explaining why it was rejected. Rejected param_sets are excluded from cache warming, so (unless VALIDATE_CURIES is off) malformed CURIEs are never sent to NodeNorm. """ - stripped = [[param.strip() for param in params] for params in param_sets] - - failures: dict[int, TestResult] = {} - for index, params in enumerate(stripped): - if not params: - failures[index] = self.failed(f"No parameters in param_set {index} in {label}") - continue - if not self.VALIDATE_CURIES: - continue - invalid = [c for c in self.curie_params(params) if not self._CURIE_RE.match(c)] - if invalid: - failures[index] = self.failed( - f"Malformed CURIE(s) {invalid} in param_set {index} in {label}: " - f"expected format PREFIX:LOCAL_ID (e.g. CHEBI:15365)" - ) + prepared = [] + for index, params in enumerate(param_sets): + stripped = [param.strip() for param in params] + prepared.append(PreparedParamSet(stripped, self._rejection(index, stripped, label))) # Warm the cache in a single request, deduplicated; skip if empty # (normalize_curies raises ValueError on an empty list). curies_to_warm = list({ - p - for index, params in enumerate(stripped) - if index not in failures - for p in self.curie_params(params) + curie + for p in prepared if p.failure is None + for curie in self.curie_params(p.params) }) if curies_to_warm: nodenorm.normalize_curies(curies_to_warm) - return stripped, failures - - def test_with_nodenorm(self, param_sets: list[list[str]], nodenorm, + return prepared + + def _rejection(self, index: int, params: Params, label: str) -> TestResult | None: + """Why *params* cannot be evaluated, or None if it can be.""" + if not params: + return self.failed(f"No parameters in param_set {index} in {label}") + if not self.VALIDATE_CURIES: + return None + invalid = [c for c in self.curie_params(params) if not self._CURIE_RE.match(c)] + if invalid: + return self.failed( + f"Malformed CURIE(s) {invalid} in param_set {index} in {label}: " + f"expected format PREFIX:LOCAL_ID (e.g. CHEBI:15365)" + ) + return None + + def test_with_nodenorm(self, param_sets: ParamSets, nodenorm, label: str = "") -> Iterator[TestResult]: """Evaluate this assertion against NodeNorm. Returns nothing if not applicable.""" return iter([]) - def test_with_nameres(self, param_sets: list[list[str]], nodenorm, nameres, + def test_with_nameres(self, param_sets: ParamSets, nodenorm, nameres, pass_if_found_in_top: int = 5, label: str = "") -> Iterator[TestResult]: """Evaluate this assertion against NameRes. Returns nothing if not applicable.""" @@ -103,24 +127,23 @@ class NodeNormTest(AssertionHandler): Subclasses implement test_param_set() instead of test_with_nodenorm(). """ - def test_with_nodenorm(self, param_sets: list[list[str]], nodenorm, + def test_with_nodenorm(self, param_sets: ParamSets, nodenorm, label: str = "") -> Iterator[TestResult]: if not param_sets: yield self.failed(f"No parameters provided in {label}") return - param_sets, failures = self.prepare_param_sets(param_sets, nodenorm, label) results = [] - for index, params in enumerate(param_sets): - if index in failures: - results.append(failures[index]) + for prepared in self.prepare_param_sets(param_sets, nodenorm, label): + if prepared.failure: + results.append(prepared.failure) continue - results.extend(self.test_param_set(params, nodenorm, label)) + results.extend(self.test_param_set(prepared.params, nodenorm, label)) if not results: yield self.failed(f"No test results returned in {label}") return yield from results - def test_param_set(self, params: list[str], nodenorm, label: str = "") -> Iterator[TestResult]: + def test_param_set(self, params: Params, nodenorm, label: str = "") -> Iterator[TestResult]: """Override this to implement the assertion. Called once per param_set.""" raise NotImplementedError @@ -146,25 +169,25 @@ class NameResTest(AssertionHandler): Subclasses implement test_param_set() instead of test_with_nameres(). """ - def test_with_nameres(self, param_sets: list[list[str]], nodenorm, nameres, + def test_with_nameres(self, param_sets: ParamSets, nodenorm, nameres, pass_if_found_in_top: int = 5, label: str = "") -> Iterator[TestResult]: if not param_sets: yield self.failed(f"No parameters provided in {label}") return - param_sets, failures = self.prepare_param_sets(param_sets, nodenorm, label) results = [] - for index, params in enumerate(param_sets): - if index in failures: - results.append(failures[index]) + for prepared in self.prepare_param_sets(param_sets, nodenorm, label): + if prepared.failure: + results.append(prepared.failure) continue - results.extend(self.test_param_set(params, nodenorm, nameres, pass_if_found_in_top, label)) + results.extend( + self.test_param_set(prepared.params, nodenorm, nameres, pass_if_found_in_top, label)) if not results: yield self.failed(f"No test results returned in {label}") return yield from results - def test_param_set(self, params: list[str], nodenorm, nameres, + def test_param_set(self, params: Params, nodenorm, nameres, pass_if_found_in_top: int, label: str = "") -> Iterator[TestResult]: """Override this to implement the assertion. Called once per param_set.""" raise NotImplementedError diff --git a/src/babel_validation/assertions/nameres.py b/src/babel_validation/assertions/nameres.py index b406a54a..cf9d10e7 100644 --- a/src/babel_validation/assertions/nameres.py +++ b/src/babel_validation/assertions/nameres.py @@ -2,7 +2,7 @@ import logging from typing import Iterator -from src.babel_validation.assertions import NameResTest +from src.babel_validation.assertions import NameResTest, Params from src.babel_validation.core.testrow import TestResult from src.babel_validation.services.nameres import CachedNameRes from src.babel_validation.services.nodenorm import CachedNodeNorm @@ -23,13 +23,13 @@ class SearchByNameHandler(NameResTest): WIKI_EXAMPLES = ["{{BabelTest|SearchByName|water|CHEBI:15377}}"] YAML_PARAMS = " - [water, CHEBI:15377]\n - [diabetes, MONDO:0005015]" - def curie_params(self, params: list[str]) -> list[str]: + def curie_params(self, params: Params) -> Params: # params[0] is a free-text search query; only the expected CURIE is a CURIE. # Slicing (rather than indexing) keeps malformed param_sets out of validation # so test_param_set() can report the arity problem instead. return params[1:2] - def test_param_set(self, params: list[str], nodenorm: CachedNodeNorm, + def test_param_set(self, params: Params, nodenorm: CachedNodeNorm, nameres: CachedNameRes, pass_if_found_in_top: int = 5, label: str = "") -> Iterator[TestResult]: if len(params) != 2: diff --git a/src/babel_validation/assertions/nodenorm.py b/src/babel_validation/assertions/nodenorm.py index 1451e7f2..1eb5e30a 100644 --- a/src/babel_validation/assertions/nodenorm.py +++ b/src/babel_validation/assertions/nodenorm.py @@ -1,6 +1,6 @@ from typing import Iterator -from src.babel_validation.assertions import NodeNormTest +from src.babel_validation.assertions import NodeNormTest, Params from src.babel_validation.core.testrow import TestResult from src.babel_validation.services.nodenorm import CachedNodeNorm @@ -16,7 +16,7 @@ class ResolvesHandler(NodeNormTest): ] YAML_PARAMS = " - CHEBI:15365\n - [MONDO:0005015, DOID:9351]" - def test_param_set(self, params: list[str], nodenorm: CachedNodeNorm, + def test_param_set(self, params: Params, nodenorm: CachedNodeNorm, label: str = "") -> Iterator[TestResult]: for curie in params: result = nodenorm.normalize_curie(curie) @@ -41,7 +41,7 @@ class DoesNotResolveHandler(NodeNormTest): # asserting that is the whole point of this assertion — so don't reject it. VALIDATE_CURIES = False - def test_param_set(self, params: list[str], nodenorm: CachedNodeNorm, + def test_param_set(self, params: Params, nodenorm: CachedNodeNorm, label: str = "") -> Iterator[TestResult]: for curie in params: result = nodenorm.normalize_curie(curie) @@ -52,7 +52,7 @@ def test_param_set(self, params: list[str], nodenorm: CachedNodeNorm, def _compare_resolutions( - params: list[str], nodenorm: CachedNodeNorm + params: Params, nodenorm: CachedNodeNorm ) -> tuple[dict | None, dict[str, dict | None]]: """Resolve all params; return (first_good_result, per_curie_results). @@ -77,7 +77,7 @@ class ResolvesWithHandler(NodeNormTest): WIKI_EXAMPLES = ["{{BabelTest|ResolvesWith|CHEBI:15365|PUBCHEM.COMPOUND:1}}"] YAML_PARAMS = " - [CHEBI:15365, PUBCHEM.COMPOUND:1]\n - [MONDO:0005015, DOID:9351]" - def test_param_set(self, params: list[str], nodenorm: CachedNodeNorm, + def test_param_set(self, params: Params, nodenorm: CachedNodeNorm, label: str = "") -> Iterator[TestResult]: if len(params) < 2: yield self.failed( @@ -123,7 +123,7 @@ class DoesNotResolveWithHandler(NodeNormTest): WIKI_EXAMPLES = ["{{BabelTest|DoesNotResolveWith|CHEBI:15365|CHEBI:16856}}"] YAML_PARAMS = " - [CHEBI:15365, CHEBI:16856]" - def test_param_set(self, params: list[str], nodenorm: CachedNodeNorm, + def test_param_set(self, params: Params, nodenorm: CachedNodeNorm, label: str = "") -> Iterator[TestResult]: if len(params) < 2: yield self.failed( @@ -176,10 +176,10 @@ class HasLabelHandler(NodeNormTest): WIKI_EXAMPLES = ["{{BabelTest|HasLabel|CHEBI:15365|aspirin}}"] YAML_PARAMS = " - [CHEBI:15365, aspirin]" - def curie_params(self, params: list[str]) -> list[str]: + def curie_params(self, params: Params) -> Params: return params[:1] - def test_param_set(self, params: list[str], nodenorm: CachedNodeNorm, + def test_param_set(self, params: Params, nodenorm: CachedNodeNorm, label: str = "") -> Iterator[TestResult]: if len(params) != 2: yield self.failed( @@ -230,10 +230,10 @@ class ResolvesWithTypeHandler(NodeNormTest): WIKI_EXAMPLES = ["{{BabelTest|ResolvesWithType|biolink:Gene|NCBIGene:1}}"] YAML_PARAMS = " - [biolink:Gene, NCBIGene:1, HGNC:5]" - def curie_params(self, params: list[str]) -> list[str]: + def curie_params(self, params: Params) -> Params: return params[1:] - def test_param_set(self, params: list[str], nodenorm: CachedNodeNorm, + def test_param_set(self, params: Params, nodenorm: CachedNodeNorm, label: str = "") -> Iterator[TestResult]: if len(params) < 2: yield self.failed(f"Too few parameters provided in param_set in {label}: {params}") From d3b157557c32e3d16b044d578b9b2ba74a1c115b Mon Sep 17 00:00:00 2001 From: Gaurav Vaidya Date: Wed, 19 Aug 2026 01:47:14 -0400 Subject: [PATCH 19/23] Rename the Params alias to ParamSet MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The package calls this concept a "param set" everywhere that matters — the generated README, every handler's PARAMETERS string, the error messages, and test_param_set(), which is the documented extension point for new assertions. Naming the alias Params while the record pairing it with a failure was called PreparedParamSet gave the same concept two names depending on which stage of the pipeline it was in. Keeps the domain term rather than renaming toward something like ParamsList: "set" here is the English sense the README already defines ("independent groups of parameters"), not Python's set type, and renaming only the alias would leave the type vocabulary at odds with the docs and the method name. Adds a comment on the alias saying so, since "set" naturally raises the question of whether order is significant — it is. Co-Authored-By: Claude Opus 5 --- src/babel_validation/assertions/__init__.py | 22 ++++++++++++--------- src/babel_validation/assertions/nameres.py | 6 +++--- src/babel_validation/assertions/nodenorm.py | 20 +++++++++---------- 3 files changed, 26 insertions(+), 22 deletions(-) diff --git a/src/babel_validation/assertions/__init__.py b/src/babel_validation/assertions/__init__.py index e945f3ea..64b44c84 100644 --- a/src/babel_validation/assertions/__init__.py +++ b/src/babel_validation/assertions/__init__.py @@ -28,11 +28,15 @@ # The parameters of a single assertion invocation, e.g. ["CHEBI:15365", "aspirin"] # for {{BabelTest|HasLabel|CHEBI:15365|aspirin}}. What each element means depends # on the assertion; see the handler's PARAMETERS attribute. -Params = list[str] +# +# "Set" is the English sense — a group of parameters evaluated together — not +# Python's set type. Order matters: ResolvesWithType takes its Biolink type +# first, HasLabel is [curie, label]. Hence list, not set. +ParamSet = list[str] -# The param_sets of one assertion: each is evaluated independently, and each -# produces its own TestResults. -ParamSets = list[Params] +# Every param_set of one assertion. Each is evaluated independently and produces +# its own TestResults, so one bad param_set doesn't sink the others. +ParamSets = list[ParamSet] @dataclass(frozen=True) @@ -43,7 +47,7 @@ class PreparedParamSet: param_set was rejected before reaching the service and *failure* is the TestResult to report in its place. """ - params: Params + params: ParamSet failure: TestResult | None = None @@ -64,7 +68,7 @@ def passed(self, message: str) -> TestResult: def failed(self, message: str) -> TestResult: return TestResult(status=TestStatus.Failed, message=message) - def curie_params(self, params: Params) -> Params: + def curie_params(self, params: ParamSet) -> ParamSet: """Return the subset of params that are CURIEs (for prewarming and validation). Default: all params are CURIEs. Subclasses override when some params are non-CURIEs.""" return params @@ -95,7 +99,7 @@ def prepare_param_sets(self, param_sets: ParamSets, nodenorm, return prepared - def _rejection(self, index: int, params: Params, label: str) -> TestResult | None: + def _rejection(self, index: int, params: ParamSet, label: str) -> TestResult | None: """Why *params* cannot be evaluated, or None if it can be.""" if not params: return self.failed(f"No parameters in param_set {index} in {label}") @@ -143,7 +147,7 @@ def test_with_nodenorm(self, param_sets: ParamSets, nodenorm, return yield from results - def test_param_set(self, params: Params, nodenorm, label: str = "") -> Iterator[TestResult]: + def test_param_set(self, params: ParamSet, nodenorm, label: str = "") -> Iterator[TestResult]: """Override this to implement the assertion. Called once per param_set.""" raise NotImplementedError @@ -187,7 +191,7 @@ def test_with_nameres(self, param_sets: ParamSets, nodenorm, nameres, return yield from results - def test_param_set(self, params: Params, nodenorm, nameres, + def test_param_set(self, params: ParamSet, nodenorm, nameres, pass_if_found_in_top: int, label: str = "") -> Iterator[TestResult]: """Override this to implement the assertion. Called once per param_set.""" raise NotImplementedError diff --git a/src/babel_validation/assertions/nameres.py b/src/babel_validation/assertions/nameres.py index cf9d10e7..b7c4d25c 100644 --- a/src/babel_validation/assertions/nameres.py +++ b/src/babel_validation/assertions/nameres.py @@ -2,7 +2,7 @@ import logging from typing import Iterator -from src.babel_validation.assertions import NameResTest, Params +from src.babel_validation.assertions import NameResTest, ParamSet from src.babel_validation.core.testrow import TestResult from src.babel_validation.services.nameres import CachedNameRes from src.babel_validation.services.nodenorm import CachedNodeNorm @@ -23,13 +23,13 @@ class SearchByNameHandler(NameResTest): WIKI_EXAMPLES = ["{{BabelTest|SearchByName|water|CHEBI:15377}}"] YAML_PARAMS = " - [water, CHEBI:15377]\n - [diabetes, MONDO:0005015]" - def curie_params(self, params: Params) -> Params: + def curie_params(self, params: ParamSet) -> ParamSet: # params[0] is a free-text search query; only the expected CURIE is a CURIE. # Slicing (rather than indexing) keeps malformed param_sets out of validation # so test_param_set() can report the arity problem instead. return params[1:2] - def test_param_set(self, params: Params, nodenorm: CachedNodeNorm, + def test_param_set(self, params: ParamSet, nodenorm: CachedNodeNorm, nameres: CachedNameRes, pass_if_found_in_top: int = 5, label: str = "") -> Iterator[TestResult]: if len(params) != 2: diff --git a/src/babel_validation/assertions/nodenorm.py b/src/babel_validation/assertions/nodenorm.py index 1eb5e30a..cda5675c 100644 --- a/src/babel_validation/assertions/nodenorm.py +++ b/src/babel_validation/assertions/nodenorm.py @@ -1,6 +1,6 @@ from typing import Iterator -from src.babel_validation.assertions import NodeNormTest, Params +from src.babel_validation.assertions import NodeNormTest, ParamSet from src.babel_validation.core.testrow import TestResult from src.babel_validation.services.nodenorm import CachedNodeNorm @@ -16,7 +16,7 @@ class ResolvesHandler(NodeNormTest): ] YAML_PARAMS = " - CHEBI:15365\n - [MONDO:0005015, DOID:9351]" - def test_param_set(self, params: Params, nodenorm: CachedNodeNorm, + def test_param_set(self, params: ParamSet, nodenorm: CachedNodeNorm, label: str = "") -> Iterator[TestResult]: for curie in params: result = nodenorm.normalize_curie(curie) @@ -41,7 +41,7 @@ class DoesNotResolveHandler(NodeNormTest): # asserting that is the whole point of this assertion — so don't reject it. VALIDATE_CURIES = False - def test_param_set(self, params: Params, nodenorm: CachedNodeNorm, + def test_param_set(self, params: ParamSet, nodenorm: CachedNodeNorm, label: str = "") -> Iterator[TestResult]: for curie in params: result = nodenorm.normalize_curie(curie) @@ -52,7 +52,7 @@ def test_param_set(self, params: Params, nodenorm: CachedNodeNorm, def _compare_resolutions( - params: Params, nodenorm: CachedNodeNorm + params: ParamSet, nodenorm: CachedNodeNorm ) -> tuple[dict | None, dict[str, dict | None]]: """Resolve all params; return (first_good_result, per_curie_results). @@ -77,7 +77,7 @@ class ResolvesWithHandler(NodeNormTest): WIKI_EXAMPLES = ["{{BabelTest|ResolvesWith|CHEBI:15365|PUBCHEM.COMPOUND:1}}"] YAML_PARAMS = " - [CHEBI:15365, PUBCHEM.COMPOUND:1]\n - [MONDO:0005015, DOID:9351]" - def test_param_set(self, params: Params, nodenorm: CachedNodeNorm, + def test_param_set(self, params: ParamSet, nodenorm: CachedNodeNorm, label: str = "") -> Iterator[TestResult]: if len(params) < 2: yield self.failed( @@ -123,7 +123,7 @@ class DoesNotResolveWithHandler(NodeNormTest): WIKI_EXAMPLES = ["{{BabelTest|DoesNotResolveWith|CHEBI:15365|CHEBI:16856}}"] YAML_PARAMS = " - [CHEBI:15365, CHEBI:16856]" - def test_param_set(self, params: Params, nodenorm: CachedNodeNorm, + def test_param_set(self, params: ParamSet, nodenorm: CachedNodeNorm, label: str = "") -> Iterator[TestResult]: if len(params) < 2: yield self.failed( @@ -176,10 +176,10 @@ class HasLabelHandler(NodeNormTest): WIKI_EXAMPLES = ["{{BabelTest|HasLabel|CHEBI:15365|aspirin}}"] YAML_PARAMS = " - [CHEBI:15365, aspirin]" - def curie_params(self, params: Params) -> Params: + def curie_params(self, params: ParamSet) -> ParamSet: return params[:1] - def test_param_set(self, params: Params, nodenorm: CachedNodeNorm, + def test_param_set(self, params: ParamSet, nodenorm: CachedNodeNorm, label: str = "") -> Iterator[TestResult]: if len(params) != 2: yield self.failed( @@ -230,10 +230,10 @@ class ResolvesWithTypeHandler(NodeNormTest): WIKI_EXAMPLES = ["{{BabelTest|ResolvesWithType|biolink:Gene|NCBIGene:1}}"] YAML_PARAMS = " - [biolink:Gene, NCBIGene:1, HGNC:5]" - def curie_params(self, params: Params) -> Params: + def curie_params(self, params: ParamSet) -> ParamSet: return params[1:] - def test_param_set(self, params: Params, nodenorm: CachedNodeNorm, + def test_param_set(self, params: ParamSet, nodenorm: CachedNodeNorm, label: str = "") -> Iterator[TestResult]: if len(params) < 2: yield self.failed(f"Too few parameters provided in param_set in {label}: {params}") From 7c5d148902b243ee2317d940ecc1e6f34d62b797 Mon Sep 17 00:00:00 2001 From: Gaurav Vaidya Date: Wed, 19 Aug 2026 01:50:03 -0400 Subject: [PATCH 20/23] Rename param_set to params_list throughout "Set" invited the reading that these are unordered and deduplicated, which they are not: position carries meaning (ResolvesWithType takes its Biolink type first, HasLabel is [curie, label]) and duplicates are evaluated as given. Nothing outside this repo depends on the vocabulary yet, so fix it before the issue parser in the next part of the stack builds on it. Renames the whole vocabulary rather than just the type alias, since a rename of only the alias would leave signatures at odds with the generated README, the handler PARAMETERS strings, the error messages users read, and the name of the extension point itself: ParamSet -> ParamsList PreparedParamSet -> PreparedParamsList prepare_param_sets-> prepare_params_lists test_param_set -> test_params_list param_set(s) -> params_list(s) (identifiers, messages, prose) Also drops the ParamSets alias in favour of spelling out list[ParamsList]; "ParamsLists" is a worse name than the nesting it hides, and the element type is now self-describing. No behaviour change. The README is regenerated and differs only in wording. Co-Authored-By: Claude Opus 5 --- src/babel_validation/assertions/README.md | 46 ++++++------ src/babel_validation/assertions/__init__.py | 81 ++++++++++----------- src/babel_validation/assertions/common.py | 4 +- src/babel_validation/assertions/gen_docs.py | 20 ++--- src/babel_validation/assertions/nameres.py | 18 ++--- src/babel_validation/assertions/nodenorm.py | 70 +++++++++--------- 6 files changed, 116 insertions(+), 123 deletions(-) diff --git a/src/babel_validation/assertions/README.md b/src/babel_validation/assertions/README.md index 65929d1e..54cec283 100644 --- a/src/babel_validation/assertions/README.md +++ b/src/babel_validation/assertions/README.md @@ -14,7 +14,7 @@ Two syntaxes are supported: {{BabelTest|AssertionType|param1|param2|...}} ``` -**YAML syntax** (multiple assertions, multiple param sets): +**YAML syntax** (multiple assertions, multiple params lists): ```` ```yaml babel_tests: @@ -26,16 +26,16 @@ babel_tests: Assertion names are case-insensitive, as is the `{{BabelTest|...}}` marker itself. -## Param Sets +## Params Lists -Each assertion can be invoked with one or more **param sets** — independent groups of +Each assertion can be invoked with one or more **params lists** — independent groups of parameters that are each evaluated separately. -- **Wiki syntax** — each `{{BabelTest|...}}` line is one param set. -- **YAML syntax** — each list entry under an assertion key is one param set; a bare string - is a single-element param set, a YAML list is a multi-element param set. +- **Wiki syntax** — each `{{BabelTest|...}}` line is one params list. +- **YAML syntax** — each list entry under an assertion key is one params list; a bare string + is a single-element params list, a YAML list is a multi-element params list. -The meaning of each element in a param set depends on the assertion type (see below). +The meaning of each element in a params list depends on the assertion type (see below). 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. @@ -49,9 +49,9 @@ These assertions test the [NodeNorm](https://nodenorm.transltr.io/docs) service. **Applies to:** NodeNorm -Each CURIE in each param_set must resolve to a non-null result in NodeNorm. +Each CURIE in each params_list must resolve to a non-null result in NodeNorm. -**Parameters:** One or more CURIEs per param_set. +**Parameters:** One or more CURIEs per params_list. **Wiki syntax:** ``` @@ -73,9 +73,9 @@ babel_tests: **Applies to:** NodeNorm -Each CURIE in each param_set must fail to resolve (return null) in NodeNorm. Use this to confirm that an identifier is intentionally not normalizable. +Each CURIE in each params_list must fail to resolve (return null) in NodeNorm. Use this to confirm that an identifier is intentionally not normalizable. -**Parameters:** One or more CURIEs per param_set. +**Parameters:** One or more CURIEs per params_list. **Wiki syntax:** ``` @@ -95,9 +95,9 @@ babel_tests: **Applies to:** NodeNorm -All CURIEs within each param_set must resolve to the identical normalized result. Use this to assert that two identifiers are equivalent. +All CURIEs within each params_list must resolve to the identical normalized result. Use this to assert that two identifiers are equivalent. -**Parameters:** Two or more CURIEs per param_set. All must resolve to the same result. +**Parameters:** Two or more CURIEs per params_list. All must resolve to the same result. **Wiki syntax:** ``` @@ -118,9 +118,9 @@ babel_tests: **Applies to:** NodeNorm -The CURIEs within each param_set must NOT all resolve to the same normalized result. Use this to assert that two identifiers are intentionally distinct entities. +The CURIEs within each params_list must NOT all resolve to the same normalized result. Use this to assert that two identifiers are intentionally distinct entities. -**Parameters:** Two or more CURIEs per param_set. They must not all resolve to the same result. +**Parameters:** Two or more CURIEs per params_list. They must not all resolve to the same result. **Wiki syntax:** ``` @@ -142,7 +142,7 @@ babel_tests: The CURIE must resolve in NodeNorm and its primary label (id.label) must match the expected label exactly (case-sensitive). -**Parameters:** Exactly two elements per param_set: a CURIE, then the expected label string. +**Parameters:** Exactly two elements per params_list: a CURIE, then the expected label string. **Wiki syntax:** ``` @@ -162,9 +162,9 @@ babel_tests: **Applies to:** NodeNorm -Each param_set must have at least two elements: the first is the expected Biolink type (e.g. 'biolink:Gene'), and the remainder are CURIEs that must resolve with that type. +Each params_list must have at least two elements: the first is the expected Biolink type (e.g. 'biolink:Gene'), and the remainder are CURIEs that must resolve with that type. -**Parameters:** Each param_set: first element is the expected Biolink type (e.g. `biolink:Gene`), remaining elements are CURIEs. +**Parameters:** Each params_list: first element is the expected Biolink type (e.g. `biolink:Gene`), remaining elements are CURIEs. **Wiki syntax:** ``` @@ -188,9 +188,9 @@ These assertions test the [NameRes](https://name-lookup.transltr.io/docs) servic **Applies to:** NameRes -Each param_set must have exactly two elements: a search query string and an expected CURIE. The test passes if the CURIE's normalized identifier appears within the top N results (default N=5) when NameRes looks up the search query. +Each params_list must have exactly two elements: a search query string and an expected CURIE. The test passes if the CURIE's normalized identifier appears within the top N results (default N=5) when NameRes looks up the search query. -**Parameters:** Each param_set: the **search query string** and the **expected CURIE**. The CURIE is normalized via NodeNorm before matching. +**Parameters:** Each params_list: the **search query string** and the **expected CURIE**. The CURIE is normalized via NodeNorm before matching. **Wiki syntax:** ``` @@ -232,11 +232,11 @@ babel_tests: ## Adding a New Assertion Type 1. Choose the right module: - - `nodenorm.py` — for NodeNorm-only assertions (subclass `NodeNormTest`, override `test_param_set`) - - `nameres.py` — for NameRes-only assertions (subclass `NameResTest`, override `test_param_set`) + - `nodenorm.py` — for NodeNorm-only assertions (subclass `NodeNormTest`, override `test_params_list`) + - `nameres.py` — for NameRes-only assertions (subclass `NameResTest`, override `test_params_list`) - `common.py` — for assertions that apply to both services (subclass `AssertionHandler`, override `test_with_nodenorm` and/or `test_with_nameres`) -2. Define the class with `NAME`, `DESCRIPTION`, `PARAMETERS`, `WIKI_EXAMPLES`, `YAML_PARAMS`, and `test_param_set()` (or both `test_with_*` methods for `AssertionHandler` subclasses). +2. Define the class with `NAME`, `DESCRIPTION`, `PARAMETERS`, `WIKI_EXAMPLES`, `YAML_PARAMS`, and `test_params_list()` (or both `test_with_*` methods for `AssertionHandler` subclasses). 3. Import it in `__init__.py` and add an instance to `ASSERTION_HANDLERS`. diff --git a/src/babel_validation/assertions/__init__.py b/src/babel_validation/assertions/__init__.py index 64b44c84..edc90f66 100644 --- a/src/babel_validation/assertions/__init__.py +++ b/src/babel_validation/assertions/__init__.py @@ -14,7 +14,7 @@ in the appropriate module (nodenorm.py, nameres.py, or common.py). 2. Set NAME and DESCRIPTION class attributes. 3. Set PARAMETERS, WIKI_EXAMPLES, and YAML_PARAMS class attributes for documentation. -4. Override test_param_set(). +4. Override test_params_list(). 5. Import it here and add an instance to ASSERTION_HANDLERS. 6. Run `uv run python -m src.babel_validation.assertions.gen_docs` to regenerate README.md. """ @@ -27,27 +27,20 @@ # The parameters of a single assertion invocation, e.g. ["CHEBI:15365", "aspirin"] # for {{BabelTest|HasLabel|CHEBI:15365|aspirin}}. What each element means depends -# on the assertion; see the handler's PARAMETERS attribute. -# -# "Set" is the English sense — a group of parameters evaluated together — not -# Python's set type. Order matters: ResolvesWithType takes its Biolink type -# first, HasLabel is [curie, label]. Hence list, not set. -ParamSet = list[str] - -# Every param_set of one assertion. Each is evaluated independently and produces -# its own TestResults, so one bad param_set doesn't sink the others. -ParamSets = list[ParamSet] +# on the assertion, and position is significant: ResolvesWithType takes its +# Biolink type first, HasLabel is [curie, label]. See the handler's PARAMETERS. +ParamsList = list[str] @dataclass(frozen=True) -class PreparedParamSet: - """One param_set after stripping and validation, ready to be evaluated. +class PreparedParamsList: + """One params_list after stripping and validation, ready to be evaluated. - *failure* is None when the param_set is usable. When it is set, the - param_set was rejected before reaching the service and *failure* is the + *failure* is None when the params_list is usable. When it is set, the + params_list was rejected before reaching the service and *failure* is the TestResult to report in its place. """ - params: ParamSet + params: ParamsList failure: TestResult | None = None @@ -68,24 +61,24 @@ def passed(self, message: str) -> TestResult: def failed(self, message: str) -> TestResult: return TestResult(status=TestStatus.Failed, message=message) - def curie_params(self, params: ParamSet) -> ParamSet: + def curie_params(self, params: ParamsList) -> ParamsList: """Return the subset of params that are CURIEs (for prewarming and validation). Default: all params are CURIEs. Subclasses override when some params are non-CURIEs.""" return params - def prepare_param_sets(self, param_sets: ParamSets, nodenorm, - label: str = "") -> list[PreparedParamSet]: - """Strip params, reject unusable param_sets, and warm the NodeNorm cache. + def prepare_params_lists(self, params_lists: list[ParamsList], nodenorm, + label: str = "") -> list[PreparedParamsList]: + """Strip params, reject unusable params_lists, and warm the NodeNorm cache. - Returns one PreparedParamSet per input param_set, in order, each either + Returns one PreparedParamsList per input params_list, in order, each either carrying stripped params or a failure explaining why it was rejected. - Rejected param_sets are excluded from cache warming, so (unless + Rejected params_lists are excluded from cache warming, so (unless VALIDATE_CURIES is off) malformed CURIEs are never sent to NodeNorm. """ prepared = [] - for index, params in enumerate(param_sets): + for index, params in enumerate(params_lists): stripped = [param.strip() for param in params] - prepared.append(PreparedParamSet(stripped, self._rejection(index, stripped, label))) + prepared.append(PreparedParamsList(stripped, self._rejection(index, stripped, label))) # Warm the cache in a single request, deduplicated; skip if empty # (normalize_curies raises ValueError on an empty list). @@ -99,26 +92,26 @@ def prepare_param_sets(self, param_sets: ParamSets, nodenorm, return prepared - def _rejection(self, index: int, params: ParamSet, label: str) -> TestResult | None: + def _rejection(self, index: int, params: ParamsList, label: str) -> TestResult | None: """Why *params* cannot be evaluated, or None if it can be.""" if not params: - return self.failed(f"No parameters in param_set {index} in {label}") + return self.failed(f"No parameters in params_list {index} in {label}") if not self.VALIDATE_CURIES: return None invalid = [c for c in self.curie_params(params) if not self._CURIE_RE.match(c)] if invalid: return self.failed( - f"Malformed CURIE(s) {invalid} in param_set {index} in {label}: " + f"Malformed CURIE(s) {invalid} in params_list {index} in {label}: " f"expected format PREFIX:LOCAL_ID (e.g. CHEBI:15365)" ) return None - def test_with_nodenorm(self, param_sets: ParamSets, nodenorm, + def test_with_nodenorm(self, params_lists: list[ParamsList], nodenorm, label: str = "") -> Iterator[TestResult]: """Evaluate this assertion against NodeNorm. Returns nothing if not applicable.""" return iter([]) - def test_with_nameres(self, param_sets: ParamSets, nodenorm, nameres, + def test_with_nameres(self, params_lists: list[ParamsList], nodenorm, nameres, pass_if_found_in_top: int = 5, label: str = "") -> Iterator[TestResult]: """Evaluate this assertion against NameRes. Returns nothing if not applicable.""" @@ -128,27 +121,27 @@ def test_with_nameres(self, param_sets: ParamSets, nodenorm, nameres, class NodeNormTest(AssertionHandler): """Base class for assertions that test NodeNorm. - Subclasses implement test_param_set() instead of test_with_nodenorm(). + Subclasses implement test_params_list() instead of test_with_nodenorm(). """ - def test_with_nodenorm(self, param_sets: ParamSets, nodenorm, + def test_with_nodenorm(self, params_lists: list[ParamsList], nodenorm, label: str = "") -> Iterator[TestResult]: - if not param_sets: + if not params_lists: yield self.failed(f"No parameters provided in {label}") return results = [] - for prepared in self.prepare_param_sets(param_sets, nodenorm, label): + for prepared in self.prepare_params_lists(params_lists, nodenorm, label): if prepared.failure: results.append(prepared.failure) continue - results.extend(self.test_param_set(prepared.params, nodenorm, label)) + results.extend(self.test_params_list(prepared.params, nodenorm, label)) if not results: yield self.failed(f"No test results returned in {label}") return yield from results - def test_param_set(self, params: ParamSet, nodenorm, label: str = "") -> Iterator[TestResult]: - """Override this to implement the assertion. Called once per param_set.""" + def test_params_list(self, params: ParamsList, nodenorm, label: str = "") -> Iterator[TestResult]: + """Override this to implement the assertion. Called once per params_list.""" raise NotImplementedError @staticmethod @@ -170,30 +163,30 @@ def resolved_message(self, curie: str, result: dict, nodenorm) -> str: class NameResTest(AssertionHandler): """Base class for assertions that test NameRes. - Subclasses implement test_param_set() instead of test_with_nameres(). + Subclasses implement test_params_list() instead of test_with_nameres(). """ - def test_with_nameres(self, param_sets: ParamSets, nodenorm, nameres, + def test_with_nameres(self, params_lists: list[ParamsList], nodenorm, nameres, pass_if_found_in_top: int = 5, label: str = "") -> Iterator[TestResult]: - if not param_sets: + if not params_lists: yield self.failed(f"No parameters provided in {label}") return results = [] - for prepared in self.prepare_param_sets(param_sets, nodenorm, label): + for prepared in self.prepare_params_lists(params_lists, nodenorm, label): if prepared.failure: results.append(prepared.failure) continue results.extend( - self.test_param_set(prepared.params, nodenorm, nameres, pass_if_found_in_top, label)) + self.test_params_list(prepared.params, nodenorm, nameres, pass_if_found_in_top, label)) if not results: yield self.failed(f"No test results returned in {label}") return yield from results - def test_param_set(self, params: ParamSet, nodenorm, nameres, - pass_if_found_in_top: int, label: str = "") -> Iterator[TestResult]: - """Override this to implement the assertion. Called once per param_set.""" + def test_params_list(self, params: ParamsList, nodenorm, nameres, + pass_if_found_in_top: int, label: str = "") -> Iterator[TestResult]: + """Override this to implement the assertion. Called once per params_list.""" raise NotImplementedError diff --git a/src/babel_validation/assertions/common.py b/src/babel_validation/assertions/common.py index ac12daca..d3166760 100644 --- a/src/babel_validation/assertions/common.py +++ b/src/babel_validation/assertions/common.py @@ -9,8 +9,8 @@ class NeededHandler(AssertionHandler): WIKI_EXAMPLES = ["{{BabelTest|Needed}}"] YAML_PARAMS = " - placeholder" - def test_with_nodenorm(self, param_sets, nodenorm, label=""): + def test_with_nodenorm(self, params_lists, nodenorm, label=""): yield self.failed("Test needed for issue") - def test_with_nameres(self, param_sets, nodenorm, nameres, pass_if_found_in_top=5, label=""): + def test_with_nameres(self, params_lists, nodenorm, nameres, pass_if_found_in_top=5, label=""): yield self.failed("Test needed for issue") diff --git a/src/babel_validation/assertions/gen_docs.py b/src/babel_validation/assertions/gen_docs.py index 36808855..450f1962 100644 --- a/src/babel_validation/assertions/gen_docs.py +++ b/src/babel_validation/assertions/gen_docs.py @@ -29,7 +29,7 @@ {{BabelTest|AssertionType|param1|param2|...}} ``` -**YAML syntax** (multiple assertions, multiple param sets): +**YAML syntax** (multiple assertions, multiple params lists): ```` ```yaml babel_tests: @@ -41,16 +41,16 @@ Assertion names are case-insensitive, as is the `{{BabelTest|...}}` marker itself. -## Param Sets +## Params Lists -Each assertion can be invoked with one or more **param sets** — independent groups of +Each assertion can be invoked with one or more **params lists** — independent groups of parameters that are each evaluated separately. -- **Wiki syntax** — each `{{BabelTest|...}}` line is one param set. -- **YAML syntax** — each list entry under an assertion key is one param set; a bare string - is a single-element param set, a YAML list is a multi-element param set. +- **Wiki syntax** — each `{{BabelTest|...}}` line is one params list. +- **YAML syntax** — each list entry under an assertion key is one params list; a bare string + is a single-element params list, a YAML list is a multi-element params list. -The meaning of each element in a param set depends on the assertion type (see below). +The meaning of each element in a params list depends on the assertion type (see below). 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. @@ -61,11 +61,11 @@ ## Adding a New Assertion Type 1. Choose the right module: - - `nodenorm.py` — for NodeNorm-only assertions (subclass `NodeNormTest`, override `test_param_set`) - - `nameres.py` — for NameRes-only assertions (subclass `NameResTest`, override `test_param_set`) + - `nodenorm.py` — for NodeNorm-only assertions (subclass `NodeNormTest`, override `test_params_list`) + - `nameres.py` — for NameRes-only assertions (subclass `NameResTest`, override `test_params_list`) - `common.py` — for assertions that apply to both services (subclass `AssertionHandler`, override `test_with_nodenorm` and/or `test_with_nameres`) -2. Define the class with `NAME`, `DESCRIPTION`, `PARAMETERS`, `WIKI_EXAMPLES`, `YAML_PARAMS`, and `test_param_set()` (or both `test_with_*` methods for `AssertionHandler` subclasses). +2. Define the class with `NAME`, `DESCRIPTION`, `PARAMETERS`, `WIKI_EXAMPLES`, `YAML_PARAMS`, and `test_params_list()` (or both `test_with_*` methods for `AssertionHandler` subclasses). 3. Import it in `__init__.py` and add an instance to `ASSERTION_HANDLERS`. diff --git a/src/babel_validation/assertions/nameres.py b/src/babel_validation/assertions/nameres.py index b7c4d25c..b9c6c73e 100644 --- a/src/babel_validation/assertions/nameres.py +++ b/src/babel_validation/assertions/nameres.py @@ -2,7 +2,7 @@ import logging from typing import Iterator -from src.babel_validation.assertions import NameResTest, ParamSet +from src.babel_validation.assertions import NameResTest, ParamsList from src.babel_validation.core.testrow import TestResult from src.babel_validation.services.nameres import CachedNameRes from src.babel_validation.services.nodenorm import CachedNodeNorm @@ -12,26 +12,26 @@ class SearchByNameHandler(NameResTest): """Test that a name search returns an expected CURIE in the top-N results in NameRes.""" NAME = "searchbyname" DESCRIPTION = ( - "Each param_set must have exactly two elements: a search query string and an expected CURIE. " + "Each params_list must have exactly two elements: a search query string and an expected CURIE. " "The test passes if the CURIE's normalized identifier appears within the top N results " "(default N=5) when NameRes looks up the search query." ) PARAMETERS = ( - "Each param_set: the **search query string** and the **expected CURIE**. " + "Each params_list: the **search query string** and the **expected CURIE**. " "The CURIE is normalized via NodeNorm before matching." ) WIKI_EXAMPLES = ["{{BabelTest|SearchByName|water|CHEBI:15377}}"] YAML_PARAMS = " - [water, CHEBI:15377]\n - [diabetes, MONDO:0005015]" - def curie_params(self, params: ParamSet) -> ParamSet: + def curie_params(self, params: ParamsList) -> ParamsList: # params[0] is a free-text search query; only the expected CURIE is a CURIE. - # Slicing (rather than indexing) keeps malformed param_sets out of validation - # so test_param_set() can report the arity problem instead. + # Slicing (rather than indexing) keeps malformed params_lists out of validation + # so test_params_list() can report the arity problem instead. return params[1:2] - def test_param_set(self, params: ParamSet, nodenorm: CachedNodeNorm, - nameres: CachedNameRes, pass_if_found_in_top: int = 5, - label: str = "") -> Iterator[TestResult]: + def test_params_list(self, params: ParamsList, nodenorm: CachedNodeNorm, + nameres: CachedNameRes, pass_if_found_in_top: int = 5, + label: str = "") -> Iterator[TestResult]: if len(params) != 2: yield self.failed( f"SearchByName requires exactly two parameters (search query, expected CURIE) in {label}, " diff --git a/src/babel_validation/assertions/nodenorm.py b/src/babel_validation/assertions/nodenorm.py index cda5675c..fabfa320 100644 --- a/src/babel_validation/assertions/nodenorm.py +++ b/src/babel_validation/assertions/nodenorm.py @@ -1,23 +1,23 @@ from typing import Iterator -from src.babel_validation.assertions import NodeNormTest, ParamSet +from src.babel_validation.assertions import NodeNormTest, ParamsList from src.babel_validation.core.testrow import TestResult from src.babel_validation.services.nodenorm import CachedNodeNorm class ResolvesHandler(NodeNormTest): - """Test that every CURIE in every param_set resolves in NodeNorm.""" + """Test that every CURIE in every params_list resolves in NodeNorm.""" NAME = "resolves" - DESCRIPTION = "Each CURIE in each param_set must resolve to a non-null result in NodeNorm." - PARAMETERS = "One or more CURIEs per param_set." + DESCRIPTION = "Each CURIE in each params_list must resolve to a non-null result in NodeNorm." + PARAMETERS = "One or more CURIEs per params_list." WIKI_EXAMPLES = [ "{{BabelTest|Resolves|CHEBI:15365}}", "{{BabelTest|Resolves|MONDO:0005015|DOID:9351}}", ] YAML_PARAMS = " - CHEBI:15365\n - [MONDO:0005015, DOID:9351]" - def test_param_set(self, params: ParamSet, nodenorm: CachedNodeNorm, - label: str = "") -> Iterator[TestResult]: + def test_params_list(self, params: ParamsList, nodenorm: CachedNodeNorm, + label: str = "") -> Iterator[TestResult]: for curie in params: result = nodenorm.normalize_curie(curie) if not result: @@ -27,13 +27,13 @@ def test_param_set(self, params: ParamSet, nodenorm: CachedNodeNorm, class DoesNotResolveHandler(NodeNormTest): - """Test that every CURIE in every param_set does NOT resolve in NodeNorm.""" + """Test that every CURIE in every params_list does NOT resolve in NodeNorm.""" NAME = "doesnotresolve" DESCRIPTION = ( - "Each CURIE in each param_set must fail to resolve (return null) in NodeNorm. " + "Each CURIE in each params_list must fail to resolve (return null) in NodeNorm. " "Use this to confirm that an identifier is intentionally not normalizable." ) - PARAMETERS = "One or more CURIEs per param_set." + PARAMETERS = "One or more CURIEs per params_list." WIKI_EXAMPLES = ["{{BabelTest|DoesNotResolve|FAKENS:99999}}"] YAML_PARAMS = " - FAKENS:99999" @@ -41,8 +41,8 @@ class DoesNotResolveHandler(NodeNormTest): # asserting that is the whole point of this assertion — so don't reject it. VALIDATE_CURIES = False - def test_param_set(self, params: ParamSet, nodenorm: CachedNodeNorm, - label: str = "") -> Iterator[TestResult]: + def test_params_list(self, params: ParamsList, nodenorm: CachedNodeNorm, + label: str = "") -> Iterator[TestResult]: for curie in params: result = nodenorm.normalize_curie(curie) if not result: @@ -52,7 +52,7 @@ def test_param_set(self, params: ParamSet, nodenorm: CachedNodeNorm, def _compare_resolutions( - params: ParamSet, nodenorm: CachedNodeNorm + params: ParamsList, nodenorm: CachedNodeNorm ) -> tuple[dict | None, dict[str, dict | None]]: """Resolve all params; return (first_good_result, per_curie_results). @@ -67,21 +67,21 @@ def _compare_resolutions( class ResolvesWithHandler(NodeNormTest): - """Test that all CURIEs in a param_set resolve to the same normalized result in NodeNorm.""" + """Test that all CURIEs in a params_list resolve to the same normalized result in NodeNorm.""" NAME = "resolveswith" DESCRIPTION = ( - "All CURIEs within each param_set must resolve to the identical normalized result. " + "All CURIEs within each params_list must resolve to the identical normalized result. " "Use this to assert that two identifiers are equivalent." ) - PARAMETERS = "Two or more CURIEs per param_set. All must resolve to the same result." + PARAMETERS = "Two or more CURIEs per params_list. All must resolve to the same result." WIKI_EXAMPLES = ["{{BabelTest|ResolvesWith|CHEBI:15365|PUBCHEM.COMPOUND:1}}"] YAML_PARAMS = " - [CHEBI:15365, PUBCHEM.COMPOUND:1]\n - [MONDO:0005015, DOID:9351]" - def test_param_set(self, params: ParamSet, nodenorm: CachedNodeNorm, - label: str = "") -> Iterator[TestResult]: + def test_params_list(self, params: ParamsList, nodenorm: CachedNodeNorm, + label: str = "") -> Iterator[TestResult]: if len(params) < 2: yield self.failed( - f"ResolvesWith requires at least two CURIEs per param_set in {label}, " + f"ResolvesWith requires at least two CURIEs per params_list in {label}, " f"but got {len(params)}: {params}" ) return @@ -113,21 +113,21 @@ def test_param_set(self, params: ParamSet, nodenorm: CachedNodeNorm, class DoesNotResolveWithHandler(NodeNormTest): - """Test that not all CURIEs in a param_set resolve to the same result in NodeNorm.""" + """Test that not all CURIEs in a params_list resolve to the same result in NodeNorm.""" NAME = "doesnotresolvewith" DESCRIPTION = ( - "The CURIEs within each param_set must NOT all resolve to the same normalized " + "The CURIEs within each params_list must NOT all resolve to the same normalized " "result. Use this to assert that two identifiers are intentionally distinct entities." ) - PARAMETERS = "Two or more CURIEs per param_set. They must not all resolve to the same result." + PARAMETERS = "Two or more CURIEs per params_list. They must not all resolve to the same result." WIKI_EXAMPLES = ["{{BabelTest|DoesNotResolveWith|CHEBI:15365|CHEBI:16856}}"] YAML_PARAMS = " - [CHEBI:15365, CHEBI:16856]" - def test_param_set(self, params: ParamSet, nodenorm: CachedNodeNorm, - label: str = "") -> Iterator[TestResult]: + def test_params_list(self, params: ParamsList, nodenorm: CachedNodeNorm, + label: str = "") -> Iterator[TestResult]: if len(params) < 2: yield self.failed( - f"DoesNotResolveWith requires at least two CURIEs per param_set in {label}, " + f"DoesNotResolveWith requires at least two CURIEs per params_list in {label}, " f"but got {len(params)}: {params}" ) return @@ -139,7 +139,7 @@ def test_param_set(self, params: ParamSet, nodenorm: CachedNodeNorm, if unresolved: yield self.failed( f"CURIEs {unresolved} could not be resolved on {nodenorm}; " - f"all CURIEs in a DoesNotResolveWith param_set must resolve" + f"all CURIEs in a DoesNotResolveWith params_list must resolve" ) return @@ -172,15 +172,15 @@ class HasLabelHandler(NodeNormTest): "The CURIE must resolve in NodeNorm and its primary label (id.label) must " "match the expected label exactly (case-sensitive)." ) - PARAMETERS = "Exactly two elements per param_set: a CURIE, then the expected label string." + PARAMETERS = "Exactly two elements per params_list: a CURIE, then the expected label string." WIKI_EXAMPLES = ["{{BabelTest|HasLabel|CHEBI:15365|aspirin}}"] YAML_PARAMS = " - [CHEBI:15365, aspirin]" - def curie_params(self, params: ParamSet) -> ParamSet: + def curie_params(self, params: ParamsList) -> ParamsList: return params[:1] - def test_param_set(self, params: ParamSet, nodenorm: CachedNodeNorm, - label: str = "") -> Iterator[TestResult]: + def test_params_list(self, params: ParamsList, nodenorm: CachedNodeNorm, + label: str = "") -> Iterator[TestResult]: if len(params) != 2: yield self.failed( f"HasLabel requires exactly two parameters (CURIE, expected label) in {label}, " @@ -220,23 +220,23 @@ class ResolvesWithTypeHandler(NodeNormTest): """Test that CURIEs resolve with a specific Biolink type in NodeNorm.""" NAME = "resolveswithtype" DESCRIPTION = ( - "Each param_set must have at least two elements: the first is the expected Biolink type " + "Each params_list must have at least two elements: the first is the expected Biolink type " "(e.g. 'biolink:Gene'), and the remainder are CURIEs that must resolve with that type." ) PARAMETERS = ( - "Each param_set: first element is the expected Biolink type (e.g. `biolink:Gene`), " + "Each params_list: first element is the expected Biolink type (e.g. `biolink:Gene`), " "remaining elements are CURIEs." ) WIKI_EXAMPLES = ["{{BabelTest|ResolvesWithType|biolink:Gene|NCBIGene:1}}"] YAML_PARAMS = " - [biolink:Gene, NCBIGene:1, HGNC:5]" - def curie_params(self, params: ParamSet) -> ParamSet: + def curie_params(self, params: ParamsList) -> ParamsList: return params[1:] - def test_param_set(self, params: ParamSet, nodenorm: CachedNodeNorm, - label: str = "") -> Iterator[TestResult]: + def test_params_list(self, params: ParamsList, nodenorm: CachedNodeNorm, + label: str = "") -> Iterator[TestResult]: if len(params) < 2: - yield self.failed(f"Too few parameters provided in param_set in {label}: {params}") + yield self.failed(f"Too few parameters provided in params_list in {label}: {params}") return expected_biolink_type = params[0] From 316444b4d8180300a65bf5005b4c6b2b20fcb35d Mon Sep 17 00:00:00 2001 From: Gaurav Vaidya Date: Wed, 19 Aug 2026 01:56:30 -0400 Subject: [PATCH 21/23] Type the service parameters and document the handler contract MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The nodenorm and nameres parameters were entirely untyped, so nothing in a signature said what they were or what a handler could call on them — the worst case being AssertionHandler.test_with_nodenorm(), whose one-line docstring described neither. Annotate them with the NodeNormService and NameResService Protocols the services modules already define for exactly this purpose ("type parameters against this Protocol rather than CachedNodeNorm directly so that a future drop-in library replacement requires no caller changes"). The concrete CachedNodeNorm/CachedNameRes annotations in the handler modules move to the Protocols too, since they were the same instruction ignored a second time. Documents the parts a new handler author has to know and could not previously find in the code: - what label is for, and that it surfaces in failure messages - what pass_if_found_in_top means, and that it also caps the NameRes request - why NameRes assertions receive NodeNorm as well (normalizing the expected CURIE so comparison is by canonical identifier, not exact string) - that yielding nothing from test_with_* means "not applicable to this service" - what test_params_list() may assume about its params (non-empty, stripped, CURIEs validated and pre-warmed) and that it should yield one result per CURIE rather than one aggregate - that handlers are shared singletons and must not hold per-evaluation state Also declares PARAMETERS, WIKI_EXAMPLES and YAML_PARAMS on the base class alongside NAME and DESCRIPTION. All five are read by gen_docs.py, so they are part of the contract, but only two were previously visible as such. Uses the :param: style already used in google_sheet_test_cases.py and conftest.py. Co-Authored-By: Claude Opus 5 --- src/babel_validation/assertions/__init__.py | 125 +++++++++++++++++--- src/babel_validation/assertions/common.py | 18 ++- src/babel_validation/assertions/gen_docs.py | 20 ++++ src/babel_validation/assertions/nameres.py | 8 +- src/babel_validation/assertions/nodenorm.py | 23 ++-- 5 files changed, 159 insertions(+), 35 deletions(-) diff --git a/src/babel_validation/assertions/__init__.py b/src/babel_validation/assertions/__init__.py index edc90f66..0e54090c 100644 --- a/src/babel_validation/assertions/__init__.py +++ b/src/babel_validation/assertions/__init__.py @@ -24,6 +24,8 @@ from typing import Iterator from src.babel_validation.core.testrow import TestResult, TestStatus +from src.babel_validation.services.nameres import NameResService +from src.babel_validation.services.nodenorm import NodeNormService # The parameters of a single assertion invocation, e.g. ["CHEBI:15365", "aspirin"] # for {{BabelTest|HasLabel|CHEBI:15365|aspirin}}. What each element means depends @@ -45,9 +47,22 @@ class PreparedParamsList: class AssertionHandler: - """Base class for all BabelTest assertion handlers.""" - NAME: str # lowercase assertion name as used in issue bodies - DESCRIPTION: str # one-line human-readable description + """Base class for all BabelTest assertion handlers. + + A handler is a stateless singleton: one instance per assertion type lives in + ASSERTION_HANDLERS and is shared by every issue being evaluated. Do not store + per-evaluation state on ``self``. + + Every handler declares the five documentation attributes below; gen_docs.py + renders README.md from them, so they are part of the handler's contract + rather than optional commentary. + """ + + NAME: str # lowercase assertion name as used in issue bodies + DESCRIPTION: str # one-line human-readable description + PARAMETERS: str # markdown describing what each param means + WIKI_EXAMPLES: list[str] # complete {{BabelTest|...}} lines, shown verbatim + YAML_PARAMS: str # indented YAML list entries for the babel_tests example # Whether CURIE params should be rejected up front if they are not well-formed. # Assertions about deliberately-invalid identifiers turn this off. @@ -56,9 +71,11 @@ class AssertionHandler: _CURIE_RE = re.compile(r'^[A-Za-z][A-Za-z0-9._-]*:[^\s]+$') def passed(self, message: str) -> TestResult: + """Build a passing TestResult. Handlers use this rather than TestResult directly.""" return TestResult(status=TestStatus.Passed, message=message) def failed(self, message: str) -> TestResult: + """Build a failing TestResult. Handlers use this rather than TestResult directly.""" return TestResult(status=TestStatus.Failed, message=message) def curie_params(self, params: ParamsList) -> ParamsList: @@ -66,12 +83,20 @@ def curie_params(self, params: ParamsList) -> ParamsList: Default: all params are CURIEs. Subclasses override when some params are non-CURIEs.""" return params - def prepare_params_lists(self, params_lists: list[ParamsList], nodenorm, + def prepare_params_lists(self, params_lists: list[ParamsList], + nodenorm: NodeNormService, label: str = "") -> list[PreparedParamsList]: """Strip params, reject unusable params_lists, and warm the NodeNorm cache. - Returns one PreparedParamsList per input params_list, in order, each either - carrying stripped params or a failure explaining why it was rejected. + :param params_lists: the params_lists to prepare, as parsed from the issue. + :param nodenorm: service whose cache is warmed with every CURIE about to be + looked up, so the per-params_list evaluation costs no further HTTP calls. + :param label: human-readable identifier for the source being evaluated (an + issue number, a test name); appears in failure messages so a reader can + tell which assertion produced them. + :returns: one PreparedParamsList per input params_list, in order, each either + carrying stripped params or a failure explaining why it was rejected. + Rejected params_lists are excluded from cache warming, so (unless VALIDATE_CURIES is off) malformed CURIEs are never sent to NodeNorm. """ @@ -106,15 +131,45 @@ def _rejection(self, index: int, params: ParamsList, label: str) -> TestResult | ) return None - def test_with_nodenorm(self, params_lists: list[ParamsList], nodenorm, + def test_with_nodenorm(self, params_lists: list[ParamsList], + nodenorm: NodeNormService, label: str = "") -> Iterator[TestResult]: - """Evaluate this assertion against NodeNorm. Returns nothing if not applicable.""" + """Evaluate this assertion against NodeNorm, yielding one TestResult per check. + + The base implementation yields nothing, which is how an assertion declares + it has no NodeNorm meaning: a caller runs every handler against both + services and an empty iterator simply contributes no results. + + :param params_lists: every params_list this assertion was invoked with; each + is evaluated independently, so one bad params_list does not sink the rest. + :param nodenorm: the NodeNorm service to evaluate against. Typically a + CachedNodeNorm for a specific deployment (dev, prod, ...), which is what + makes the same assertion runnable against several environments. + :param label: human-readable identifier for the source being evaluated; see + prepare_params_lists(). + """ return iter([]) - def test_with_nameres(self, params_lists: list[ParamsList], nodenorm, nameres, + def test_with_nameres(self, params_lists: list[ParamsList], + nodenorm: NodeNormService, nameres: NameResService, pass_if_found_in_top: int = 5, label: str = "") -> Iterator[TestResult]: - """Evaluate this assertion against NameRes. Returns nothing if not applicable.""" + """Evaluate this assertion against NameRes, yielding one TestResult per check. + + As with test_with_nodenorm(), yielding nothing means "not applicable". + + NameRes assertions get *both* services: NameRes answers the lookup, and + NodeNorm normalizes the expected CURIE so that a lookup result can be + compared against it by canonical identifier rather than by exact string. + + :param params_lists: every params_list this assertion was invoked with. + :param nodenorm: used to normalize expected CURIEs before comparison. + :param nameres: the NameRes service to evaluate against. + :param pass_if_found_in_top: how far down the ranked results the expected + CURIE may appear and still count as a pass. Also caps the number of + results requested from NameRes. + :param label: human-readable identifier for the source being evaluated. + """ return iter([]) @@ -124,7 +179,8 @@ class NodeNormTest(AssertionHandler): Subclasses implement test_params_list() instead of test_with_nodenorm(). """ - def test_with_nodenorm(self, params_lists: list[ParamsList], nodenorm, + def test_with_nodenorm(self, params_lists: list[ParamsList], + nodenorm: NodeNormService, label: str = "") -> Iterator[TestResult]: if not params_lists: yield self.failed(f"No parameters provided in {label}") @@ -140,8 +196,23 @@ def test_with_nodenorm(self, params_lists: list[ParamsList], nodenorm, return yield from results - def test_params_list(self, params: ParamsList, nodenorm, label: str = "") -> Iterator[TestResult]: - """Override this to implement the assertion. Called once per params_list.""" + def test_params_list(self, params: ParamsList, nodenorm: NodeNormService, + label: str = "") -> Iterator[TestResult]: + """Override this to implement the assertion. Called once per params_list. + + *params* is non-empty and already stripped, and (unless VALIDATE_CURIES is + off) every param that curie_params() selects is a well-formed CURIE, so + implementations need only check assertion-specific shape such as arity. + Every CURIE is also pre-warmed in *nodenorm*'s cache, so normalize_curie() + calls here are free. + + Yield one TestResult per thing checked — usually one per CURIE — rather + than a single aggregate, so a failure report names the CURIE that failed. + + :param params: this params_list's parameters; see the handler's PARAMETERS. + :param nodenorm: the NodeNorm service to evaluate against. + :param label: human-readable identifier for the source being evaluated. + """ raise NotImplementedError @staticmethod @@ -153,8 +224,13 @@ def first_type(result: dict) -> str: types = result.get('type') or [] return types[0] if types else 'unknown type' - def resolved_message(self, curie: str, result: dict, nodenorm) -> str: - """Standard pass-message when a CURIE resolves.""" + def resolved_message(self, curie: str, result: dict, + nodenorm: NodeNormService) -> str: + """Standard pass-message when a CURIE resolves. + + *result* is one entry of a NodeNorm get_normalized_nodes response, i.e. a + non-None value from normalize_curie()/normalize_curies(). + """ return (f"Resolved {curie} to {result['id']['identifier']} " f"({self.first_type(result)}, \"{result['id'].get('label', '')}\") " f"with NodeNormalization service {nodenorm}") @@ -166,7 +242,8 @@ class NameResTest(AssertionHandler): Subclasses implement test_params_list() instead of test_with_nameres(). """ - def test_with_nameres(self, params_lists: list[ParamsList], nodenorm, nameres, + def test_with_nameres(self, params_lists: list[ParamsList], + nodenorm: NodeNormService, nameres: NameResService, pass_if_found_in_top: int = 5, label: str = "") -> Iterator[TestResult]: if not params_lists: @@ -184,9 +261,16 @@ def test_with_nameres(self, params_lists: list[ParamsList], nodenorm, nameres, return yield from results - def test_params_list(self, params: ParamsList, nodenorm, nameres, - pass_if_found_in_top: int, label: str = "") -> Iterator[TestResult]: - """Override this to implement the assertion. Called once per params_list.""" + def test_params_list(self, params: ParamsList, nodenorm: NodeNormService, + nameres: NameResService, pass_if_found_in_top: int, + label: str = "") -> Iterator[TestResult]: + """Override this to implement the assertion. Called once per params_list. + + *params* is non-empty and already stripped, with the params that + curie_params() selects validated as CURIEs and pre-warmed in *nodenorm*'s + cache. See NodeNormTest.test_params_list() for the shared contract; the + arguments are documented on test_with_nameres(). + """ raise NotImplementedError @@ -198,6 +282,9 @@ def test_params_list(self, params: ParamsList, nodenorm, nameres, from src.babel_validation.assertions.nameres import SearchByNameHandler # noqa: E402 from src.babel_validation.assertions.common import NeededHandler # noqa: E402 +# Every assertion type the parser will recognise, keyed by its lowercase NAME. +# Registration order is irrelevant — README.md groups handlers by the service they +# test, not by their position here. ASSERTION_HANDLERS: dict[str, AssertionHandler] = { h.NAME: h for h in [ ResolvesHandler(), diff --git a/src/babel_validation/assertions/common.py b/src/babel_validation/assertions/common.py index d3166760..a170386e 100644 --- a/src/babel_validation/assertions/common.py +++ b/src/babel_validation/assertions/common.py @@ -1,4 +1,9 @@ -from src.babel_validation.assertions import AssertionHandler +from typing import Iterator + +from src.babel_validation.assertions import AssertionHandler, ParamsList +from src.babel_validation.core.testrow import TestResult +from src.babel_validation.services.nameres import NameResService +from src.babel_validation.services.nodenorm import NodeNormService class NeededHandler(AssertionHandler): @@ -9,8 +14,15 @@ class NeededHandler(AssertionHandler): WIKI_EXAMPLES = ["{{BabelTest|Needed}}"] YAML_PARAMS = " - placeholder" - def test_with_nodenorm(self, params_lists, nodenorm, label=""): + # Applies to both services, and ignores its params entirely: the assertion + # records that a test is missing, so there is nothing to evaluate. + def test_with_nodenorm(self, params_lists: list[ParamsList], + nodenorm: NodeNormService, + label: str = "") -> Iterator[TestResult]: yield self.failed("Test needed for issue") - def test_with_nameres(self, params_lists, nodenorm, nameres, pass_if_found_in_top=5, label=""): + def test_with_nameres(self, params_lists: list[ParamsList], + nodenorm: NodeNormService, nameres: NameResService, + pass_if_found_in_top: int = 5, + label: str = "") -> Iterator[TestResult]: yield self.failed("Test needed for issue") diff --git a/src/babel_validation/assertions/gen_docs.py b/src/babel_validation/assertions/gen_docs.py index 450f1962..7fd780da 100644 --- a/src/babel_validation/assertions/gen_docs.py +++ b/src/babel_validation/assertions/gen_docs.py @@ -86,10 +86,20 @@ def _display_name(h: AssertionHandler) -> str: + """The assertion name as written in issues (ResolvesHandler -> "Resolves"). + + Derived from the class name rather than NAME, which is lowercased for + case-insensitive matching and so reads poorly as a heading. + """ return type(h).__name__.removesuffix("Handler") def _applies_to(h: AssertionHandler) -> str: + """Which service(s) this handler tests; also the key into _GROUP_HEADERS. + + A handler that subclasses neither base overrides the test_with_* methods + directly and so applies to both. + """ if isinstance(h, NodeNormTest): return "NodeNorm" if isinstance(h, NameResTest): @@ -98,6 +108,11 @@ def _applies_to(h: AssertionHandler) -> str: def _render_handler(h: AssertionHandler) -> str: + """Render one handler's README section from its documentation attributes. + + Reads them with getattr defaults so that a handler missing one still renders + (as an empty section) instead of breaking the whole README. + """ name = _display_name(h) service = _applies_to(h) description = getattr(h, "DESCRIPTION", "") @@ -126,6 +141,11 @@ def _render_handler(h: AssertionHandler) -> str: def generate_readme() -> str: + """Render the complete README.md content. Pure — writing it is the caller's job. + + Kept side-effect free so test_assertions_docs.py can compare the rendered + output against the checked-in file without touching the filesystem. + """ sections = [INTRO] # Group by service rather than by registration order, so a handler added diff --git a/src/babel_validation/assertions/nameres.py b/src/babel_validation/assertions/nameres.py index b9c6c73e..19d9f99d 100644 --- a/src/babel_validation/assertions/nameres.py +++ b/src/babel_validation/assertions/nameres.py @@ -4,8 +4,8 @@ from src.babel_validation.assertions import NameResTest, ParamsList from src.babel_validation.core.testrow import TestResult -from src.babel_validation.services.nameres import CachedNameRes -from src.babel_validation.services.nodenorm import CachedNodeNorm +from src.babel_validation.services.nameres import NameResService +from src.babel_validation.services.nodenorm import NodeNormService class SearchByNameHandler(NameResTest): @@ -29,8 +29,8 @@ def curie_params(self, params: ParamsList) -> ParamsList: # so test_params_list() can report the arity problem instead. return params[1:2] - def test_params_list(self, params: ParamsList, nodenorm: CachedNodeNorm, - nameres: CachedNameRes, pass_if_found_in_top: int = 5, + def test_params_list(self, params: ParamsList, nodenorm: NodeNormService, + nameres: NameResService, pass_if_found_in_top: int = 5, label: str = "") -> Iterator[TestResult]: if len(params) != 2: yield self.failed( diff --git a/src/babel_validation/assertions/nodenorm.py b/src/babel_validation/assertions/nodenorm.py index fabfa320..1e6f0306 100644 --- a/src/babel_validation/assertions/nodenorm.py +++ b/src/babel_validation/assertions/nodenorm.py @@ -2,7 +2,7 @@ from src.babel_validation.assertions import NodeNormTest, ParamsList from src.babel_validation.core.testrow import TestResult -from src.babel_validation.services.nodenorm import CachedNodeNorm +from src.babel_validation.services.nodenorm import NodeNormService class ResolvesHandler(NodeNormTest): @@ -16,7 +16,7 @@ class ResolvesHandler(NodeNormTest): ] YAML_PARAMS = " - CHEBI:15365\n - [MONDO:0005015, DOID:9351]" - def test_params_list(self, params: ParamsList, nodenorm: CachedNodeNorm, + def test_params_list(self, params: ParamsList, nodenorm: NodeNormService, label: str = "") -> Iterator[TestResult]: for curie in params: result = nodenorm.normalize_curie(curie) @@ -41,7 +41,7 @@ class DoesNotResolveHandler(NodeNormTest): # asserting that is the whole point of this assertion — so don't reject it. VALIDATE_CURIES = False - def test_params_list(self, params: ParamsList, nodenorm: CachedNodeNorm, + def test_params_list(self, params: ParamsList, nodenorm: NodeNormService, label: str = "") -> Iterator[TestResult]: for curie in params: result = nodenorm.normalize_curie(curie) @@ -52,11 +52,16 @@ def test_params_list(self, params: ParamsList, nodenorm: CachedNodeNorm, def _compare_resolutions( - params: ParamsList, nodenorm: CachedNodeNorm + params: ParamsList, nodenorm: NodeNormService ) -> tuple[dict | None, dict[str, dict | None]]: """Resolve all params; return (first_good_result, per_curie_results). - first_good_result is None if every CURIE failed to resolve. + Shared by ResolvesWith and DoesNotResolveWith, which ask the same question + (do these CURIEs agree?) and differ only in which answer they expect. + + first_good_result is None if every CURIE failed to resolve; otherwise it is + the result of the earliest param that resolved, and serves as the canonical + result the others are compared against. per_curie_results maps each CURIE to its result (None if unresolvable). """ # normalize_curies() guarantees one entry per requested CURIE, in the order @@ -77,7 +82,7 @@ class ResolvesWithHandler(NodeNormTest): WIKI_EXAMPLES = ["{{BabelTest|ResolvesWith|CHEBI:15365|PUBCHEM.COMPOUND:1}}"] YAML_PARAMS = " - [CHEBI:15365, PUBCHEM.COMPOUND:1]\n - [MONDO:0005015, DOID:9351]" - def test_params_list(self, params: ParamsList, nodenorm: CachedNodeNorm, + def test_params_list(self, params: ParamsList, nodenorm: NodeNormService, label: str = "") -> Iterator[TestResult]: if len(params) < 2: yield self.failed( @@ -123,7 +128,7 @@ class DoesNotResolveWithHandler(NodeNormTest): WIKI_EXAMPLES = ["{{BabelTest|DoesNotResolveWith|CHEBI:15365|CHEBI:16856}}"] YAML_PARAMS = " - [CHEBI:15365, CHEBI:16856]" - def test_params_list(self, params: ParamsList, nodenorm: CachedNodeNorm, + def test_params_list(self, params: ParamsList, nodenorm: NodeNormService, label: str = "") -> Iterator[TestResult]: if len(params) < 2: yield self.failed( @@ -179,7 +184,7 @@ class HasLabelHandler(NodeNormTest): def curie_params(self, params: ParamsList) -> ParamsList: return params[:1] - def test_params_list(self, params: ParamsList, nodenorm: CachedNodeNorm, + def test_params_list(self, params: ParamsList, nodenorm: NodeNormService, label: str = "") -> Iterator[TestResult]: if len(params) != 2: yield self.failed( @@ -233,7 +238,7 @@ class ResolvesWithTypeHandler(NodeNormTest): def curie_params(self, params: ParamsList) -> ParamsList: return params[1:] - def test_params_list(self, params: ParamsList, nodenorm: CachedNodeNorm, + def test_params_list(self, params: ParamsList, nodenorm: NodeNormService, label: str = "") -> Iterator[TestResult]: if len(params) < 2: yield self.failed(f"Too few parameters provided in params_list in {label}: {params}") From d2f684c8de123c2445d2d8c377d5af106af8f415 Mon Sep 17 00:00:00 2001 From: Gaurav Vaidya Date: Wed, 19 Aug 2026 02:14:18 -0400 Subject: [PATCH 22/23] Make the missing-Biolink-type placeholder unmistakable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 'unknown type' had the shape of a real Biolink type: current ones are prefixed ("biolink:Gene"), but the older vocabulary NodeNorm used was lowercase prose ("chemical entity"), so a reader scanning a failure message could reasonably take 'unknown type' for something Babel actually returned. Use NO TYPE RETURNED instead, named as NodeNormTest.NO_TYPE so a test can assert on it without restating the literal. Wording covers both branches: first_type() falls back when the type list is empty *and* when the key is missing altogether, so "empty type list" would have been wrong in the second case. Adds a test, since this branch had no coverage — it checks the placeholder can't be confused with either type vocabulary rather than just comparing to the constant, which would pass for any value at all. Co-Authored-By: Claude Opus 5 --- src/babel_validation/assertions/__init__.py | 10 ++++++++-- tests/test_environment/test_assertions.py | 12 ++++++++++++ 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/src/babel_validation/assertions/__init__.py b/src/babel_validation/assertions/__init__.py index 0e54090c..ace787d5 100644 --- a/src/babel_validation/assertions/__init__.py +++ b/src/babel_validation/assertions/__init__.py @@ -215,14 +215,20 @@ def test_params_list(self, params: ParamsList, nodenorm: NodeNormService, """ raise NotImplementedError + # Stand-in for the Biolink type in a message when NodeNorm returned none. + # Deliberately unlike any real type: current ones are prefixed ("biolink:Gene") + # and older ones were lowercase prose ("chemical entity"), so shouting it in + # caps keeps a reader from mistaking the placeholder for a type Babel returned. + NO_TYPE = 'NO TYPE RETURNED' + @staticmethod def first_type(result: dict) -> str: - """First Biolink type of a resolved node, or a placeholder if the node has none. + """First Biolink type of a resolved node, or NO_TYPE if the node has none. NodeNorm normally returns a non-empty `type` list, but guard against an empty (or missing) one so message formatting never raises IndexError/KeyError.""" types = result.get('type') or [] - return types[0] if types else 'unknown type' + return types[0] if types else NodeNormTest.NO_TYPE def resolved_message(self, curie: str, result: dict, nodenorm: NodeNormService) -> str: diff --git a/tests/test_environment/test_assertions.py b/tests/test_environment/test_assertions.py index 704e0a5b..effc8aa9 100644 --- a/tests/test_environment/test_assertions.py +++ b/tests/test_environment/test_assertions.py @@ -129,3 +129,15 @@ def test_docs_group_handlers_by_service_not_registration_order(): del ASSERTION_HANDLERS[TempGroupingHandler.NAME] assert '### TempGrouping' in readme assert readme.index('### TempGrouping') < readme.index('## NameRes Assertions') + + +@pytest.mark.unit +def test_missing_biolink_type_placeholder_cannot_pass_for_a_real_type(): + """Nodes do carry types normally, but the stand-in must not read as one of them.""" + assert NodeNormTest.first_type({'type': ['biolink:Gene']}) == 'biolink:Gene' + for typeless in ({}, {'type': []}, {'type': None}): + placeholder = NodeNormTest.first_type(typeless) + assert placeholder == NodeNormTest.NO_TYPE + # Unlike a current type (biolink:Gene) or a legacy one (chemical entity). + assert not placeholder.startswith('biolink:') + assert placeholder.isupper() From 347f16588d3254614e7c75c1ec4e28b45223c43b Mon Sep 17 00:00:00 2001 From: Gaurav Vaidya Date: Wed, 19 Aug 2026 02:18:42 -0400 Subject: [PATCH 23/23] Enforce the NAME rules at registration and consolidate the how-to MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three separate comments said NAME must be lowercase and nothing checked it. The rule is load-bearing: README.md promises users that assertion names are case-insensitive, which only holds if every registry key is already lowercase, so a handler declaring NAME = "Resolves" would become unreachable the moment the issue parser lowercases its input. Building the registry with a dict comprehension had a second silent failure mode — two handlers sharing a NAME would drop one with no error. Replace the comprehension with _register(), which rejects both. Neither can happen except while adding an assertion, so failing at import is the cheapest possible feedback. Tests cover both rejections and assert the real registry satisfies them. Also folds the "adding a new assertion" instructions into one place. There were two: a six-step list in this module's docstring and gen_docs.ADDING_NEW, which renders into assertions/README.md. They had already drifted — the docstring split the attributes across two steps and never mentioned the lowercase rule. README.md keeps the instructions, since it is what someone adding an assertion is already reading; the docstring now points at it and describes the module layout instead, which is what a code reader wants. The README section also gains what a new author previously had to infer from the base classes: what each documentation attribute is for, what test_params_list() may assume about its params, and when to override curie_params(). Co-Authored-By: Claude Opus 5 --- src/babel_validation/assertions/README.md | 29 +++++++-- src/babel_validation/assertions/__init__.py | 72 ++++++++++++++------- src/babel_validation/assertions/gen_docs.py | 29 +++++++-- tests/test_environment/test_assertions.py | 25 ++++++- 4 files changed, 121 insertions(+), 34 deletions(-) diff --git a/src/babel_validation/assertions/README.md b/src/babel_validation/assertions/README.md index 54cec283..c52712cb 100644 --- a/src/babel_validation/assertions/README.md +++ b/src/babel_validation/assertions/README.md @@ -236,8 +236,27 @@ babel_tests: - `nameres.py` — for NameRes-only assertions (subclass `NameResTest`, override `test_params_list`) - `common.py` — for assertions that apply to both services (subclass `AssertionHandler`, override `test_with_nodenorm` and/or `test_with_nameres`) -2. Define the class with `NAME`, `DESCRIPTION`, `PARAMETERS`, `WIKI_EXAMPLES`, `YAML_PARAMS`, and `test_params_list()` (or both `test_with_*` methods for `AssertionHandler` subclasses). - -3. Import it in `__init__.py` and add an instance to `ASSERTION_HANDLERS`. - -4. Run `uv run python -m src.babel_validation.assertions.gen_docs` to regenerate `README.md`. +2. Give the class its five documentation attributes: + - `NAME` — **must be all lowercase.** Assertions are matched case-insensitively by + lowercasing whatever the issue wrote, so a `NAME` containing any uppercase could + never be matched. Registration rejects it rather than letting it fail silently. + - `DESCRIPTION` — one line, shown under the heading here. + - `PARAMETERS` — what each element of a params_list means, and how many are expected. + - `WIKI_EXAMPLES` — complete `{{BabelTest|...}}` lines, reproduced verbatim. + - `YAML_PARAMS` — indented list entries for the YAML example. + + 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`. + +4. 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`, + and `uv run pytest -m unit` to confirm the checked-in copy is in sync. diff --git a/src/babel_validation/assertions/__init__.py b/src/babel_validation/assertions/__init__.py index ace787d5..a99b85be 100644 --- a/src/babel_validation/assertions/__init__.py +++ b/src/babel_validation/assertions/__init__.py @@ -6,17 +6,21 @@ and evaluated against the NodeNorm and NameRes services. Supported assertion types are registered in ASSERTION_HANDLERS. To see everything -that is currently supported, scan that dict or read assertions/README.md (auto-generated). - -Adding a new assertion type ---------------------------- -1. Create a subclass of NodeNormTest or NameResTest (or AssertionHandler for both) - in the appropriate module (nodenorm.py, nameres.py, or common.py). -2. Set NAME and DESCRIPTION class attributes. -3. Set PARAMETERS, WIKI_EXAMPLES, and YAML_PARAMS class attributes for documentation. -4. Override test_params_list(). -5. Import it here and add an instance to ASSERTION_HANDLERS. -6. Run `uv run python -m src.babel_validation.assertions.gen_docs` to regenerate README.md. +that is currently supported, scan that dict or read assertions/README.md. + +**Adding a new assertion type: see the "Adding a New Assertion Type" section of +assertions/README.md.** That section is generated from gen_docs.ADDING_NEW, and is +the one place those instructions live — a second copy here would drift out of step +with it, which is exactly what happened to the copy this note replaced. + +The layout, for orientation while reading the code: + +- AssertionHandler — the base class, and the strip/validate/warm machinery every + assertion shares (prepare_params_lists). +- NodeNormTest / NameResTest — specialize it per service; subclasses override + test_params_list() and are handed one params_list at a time. +- nodenorm.py, nameres.py, common.py — the concrete handlers. +- gen_docs.py — renders README.md from the handler classes. """ import re @@ -288,18 +292,40 @@ def test_params_list(self, params: ParamsList, nodenorm: NodeNormService, from src.babel_validation.assertions.nameres import SearchByNameHandler # noqa: E402 from src.babel_validation.assertions.common import NeededHandler # noqa: E402 +def _register(handlers: list[AssertionHandler]) -> dict[str, AssertionHandler]: + """Index *handlers* by NAME, rejecting what a dict comprehension would hide. + + Assertion names are matched case-insensitively by lowercasing the name used + in the issue, so a NAME that is not already lowercase can never be looked up. + A duplicate NAME would silently drop one of the two handlers. Both are + mistakes only made while adding an assertion, so fail loudly at import. + """ + registry: dict[str, AssertionHandler] = {} + for handler in handlers: + name = handler.NAME + if not name or name != name.lower(): + raise ValueError( + f"{type(handler).__name__}.NAME must be a non-empty lowercase string, got {name!r}" + ) + if name in registry: + raise ValueError( + f"{type(handler).__name__}.NAME {name!r} is already registered " + f"by {type(registry[name]).__name__}" + ) + registry[name] = handler + return registry + + # Every assertion type the parser will recognise, keyed by its lowercase NAME. # Registration order is irrelevant — README.md groups handlers by the service they # test, not by their position here. -ASSERTION_HANDLERS: dict[str, AssertionHandler] = { - h.NAME: h for h in [ - ResolvesHandler(), - DoesNotResolveHandler(), - ResolvesWithHandler(), - DoesNotResolveWithHandler(), - HasLabelHandler(), - ResolvesWithTypeHandler(), - SearchByNameHandler(), - NeededHandler(), - ] -} +ASSERTION_HANDLERS: dict[str, AssertionHandler] = _register([ + ResolvesHandler(), + DoesNotResolveHandler(), + ResolvesWithHandler(), + DoesNotResolveWithHandler(), + HasLabelHandler(), + ResolvesWithTypeHandler(), + SearchByNameHandler(), + NeededHandler(), +]) diff --git a/src/babel_validation/assertions/gen_docs.py b/src/babel_validation/assertions/gen_docs.py index 7fd780da..3d4eb413 100644 --- a/src/babel_validation/assertions/gen_docs.py +++ b/src/babel_validation/assertions/gen_docs.py @@ -65,11 +65,30 @@ - `nameres.py` — for NameRes-only assertions (subclass `NameResTest`, override `test_params_list`) - `common.py` — for assertions that apply to both services (subclass `AssertionHandler`, override `test_with_nodenorm` and/or `test_with_nameres`) -2. Define the class with `NAME`, `DESCRIPTION`, `PARAMETERS`, `WIKI_EXAMPLES`, `YAML_PARAMS`, and `test_params_list()` (or both `test_with_*` methods for `AssertionHandler` subclasses). - -3. Import it in `__init__.py` and add an instance to `ASSERTION_HANDLERS`. - -4. Run `uv run python -m src.babel_validation.assertions.gen_docs` to regenerate `README.md`. +2. Give the class its five documentation attributes: + - `NAME` — **must be all lowercase.** Assertions are matched case-insensitively by + lowercasing whatever the issue wrote, so a `NAME` containing any uppercase could + never be matched. Registration rejects it rather than letting it fail silently. + - `DESCRIPTION` — one line, shown under the heading here. + - `PARAMETERS` — what each element of a params_list means, and how many are expected. + - `WIKI_EXAMPLES` — complete `{{BabelTest|...}}` lines, reproduced verbatim. + - `YAML_PARAMS` — indented list entries for the YAML example. + + 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`. + +4. 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`, + and `uv run pytest -m unit` to confirm the checked-in copy is in sync. """ _GROUP_HEADERS: dict[str, str] = { diff --git a/tests/test_environment/test_assertions.py b/tests/test_environment/test_assertions.py index effc8aa9..9359b888 100644 --- a/tests/test_environment/test_assertions.py +++ b/tests/test_environment/test_assertions.py @@ -6,7 +6,7 @@ import pytest -from src.babel_validation.assertions import ASSERTION_HANDLERS, NodeNormTest +from src.babel_validation.assertions import ASSERTION_HANDLERS, NodeNormTest, _register from src.babel_validation.assertions.gen_docs import generate_readme from src.babel_validation.assertions.nodenorm import ( DoesNotResolveHandler, DoesNotResolveWithHandler, ResolvesHandler, ResolvesWithHandler, @@ -141,3 +141,26 @@ def test_missing_biolink_type_placeholder_cannot_pass_for_a_real_type(): # Unlike a current type (biolink:Gene) or a legacy one (chemical entity). assert not placeholder.startswith('biolink:') assert placeholder.isupper() + + +@pytest.mark.unit +def test_registration_rejects_names_that_could_never_be_matched(): + """Assertion lookup lowercases the issue's name, so an uppercase NAME is unreachable.""" + class UppercaseHandler(TempGroupingHandler): + NAME = 'Resolves' + + with pytest.raises(ValueError, match='lowercase'): + _register([UppercaseHandler()]) + + +@pytest.mark.unit +def test_registration_rejects_a_duplicate_name(): + """A dict comprehension would silently drop one of the two handlers.""" + with pytest.raises(ValueError, match='already registered'): + _register([TempGroupingHandler(), TempGroupingHandler()]) + + +@pytest.mark.unit +def test_registered_handlers_satisfy_those_rules(): + assert all(name == name.lower() for name in ASSERTION_HANDLERS) + assert len(ASSERTION_HANDLERS) == 8