Skip to content

test: re-enable base fixture validation; harden AAC tier validator - #79

Merged
korikuzma merged 2 commits into
ballot-updatesfrom
fix-base-fixture-coverage
Sep 30, 2026
Merged

korikuzma merged 2 commits into
ballot-updatesfrom
fix-base-fixture-coverage

Conversation

@larrybabb

Copy link
Copy Markdown
Contributor

Addresses two review findings from #78. Targets the ballot-updates branch so the fixes land in that PR.

1. Base-namespace fixtures were silently unvalidated

The va-spec 1.1.0-ballot.2026-09 submodule labels core/base test fixtures with the namespace va-spec (no dot); profile fixtures use va-spec.<profile>. The fixture-discovery loop in test_va_spec_fixtures_validation.py only kept namespaces matching startswith("va-spec."), so all 39 base fixtures were dropped and test_va_spec_fixtures validated only the 20 profile fixtures — while still passing green, hiding the loss of coverage.

Fix: accept the bare va-spec namespace (routed to VaSpecSchema.BASE) and continue past any namespace get_va_spec_schema doesn't recognize (also removes a latent KeyError on VA_SPEC_TEST_DEFINITIONS[None]).

Routing after the fix (verified against the pinned submodule's test_definitions.yaml):

schema fixtures
BASE 39 (was 0)
AAC_2017 11
ACMG_2015 5
CCV_2022 4
skipped (vrs) 2

2. AAC validate_tier_evidence_lines crashed on non-dict input

VariantClinicalSignificanceStatement.validate_tier_evidence_lines runs in pydantic mode="before" and called values.get(...) unconditionally, so validating from a non-dict input (e.g. model_validate("propositions.json#/1")) raised AttributeError instead of a normal validation error. Added an isinstance(values, dict) passthrough guard, matching the existing nested type guards in the same validator.

Verification

  • ruff / ruff-format pre-commit hooks pass on both files.
  • Fixture routing table above reproduced with the real submodule namespaces.
  • Non-dict guard exercised (str / list / None pass through; dict still processed).

Note: the local environment has stale dependency versions (vrs 2.0.0 / cat_vrs 0.5.0, no ga4gh.core.metadata), so the re-enabled base fixtures could not be executed here — CI on the updated deps should confirm they validate.

🤖 Generated with Claude Code

larrybabb and others added 2 commits September 30, 2026 17:13
The va-spec 1.1.0-ballot.2026-09 submodule labels core/base test
fixtures with the namespace ``va-spec`` (no dot), whereas profile
fixtures use ``va-spec.<profile>``. The fixture-discovery loop only
processed namespaces matching ``startswith("va-spec.")``, so all 39
base fixtures were silently dropped and ``test_va_spec_fixtures``
validated only the 20 profile fixtures while still passing green.

Accept the bare ``va-spec`` namespace so base fixtures are routed to
VaSpecSchema.BASE and validated again, and skip (rather than KeyError
on) any namespace that ``get_va_spec_schema`` does not recognize.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
``validate_tier_evidence_lines`` runs in pydantic ``mode="before"`` and
immediately called ``values.get(...)``. When the model is validated from
a non-dict input (e.g. ``VariantClinicalSignificanceStatement.model_validate``
called with a string/list/other object), that raised ``AttributeError``
instead of surfacing a normal validation error.

Return non-dict input unchanged so pydantic handles it through its usual
path, matching the pattern used by the nested ``classification``/``primaryCoding``
type guards already in this validator.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@korikuzma
korikuzma merged commit ee2954d into ballot-updates Sep 30, 2026
8 checks passed
@korikuzma
korikuzma deleted the fix-base-fixture-coverage branch September 30, 2026 09:51
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants