Repository navigation
test: re-enable base fixture validation; harden AAC tier validator - #79
Merged
Merged
Conversation
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>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Addresses two review findings from #78. Targets the
ballot-updatesbranch so the fixes land in that PR.1. Base-namespace fixtures were silently unvalidated
The va-spec
1.1.0-ballot.2026-09submodule labels core/base test fixtures with the namespaceva-spec(no dot); profile fixtures useva-spec.<profile>. The fixture-discovery loop intest_va_spec_fixtures_validation.pyonly kept namespaces matchingstartswith("va-spec."), so all 39 base fixtures were dropped andtest_va_spec_fixturesvalidated only the 20 profile fixtures — while still passing green, hiding the loss of coverage.Fix: accept the bare
va-specnamespace (routed toVaSpecSchema.BASE) andcontinuepast any namespaceget_va_spec_schemadoesn't recognize (also removes a latentKeyErroronVA_SPEC_TEST_DEFINITIONS[None]).Routing after the fix (verified against the pinned submodule's
test_definitions.yaml):vrs)2. AAC
validate_tier_evidence_linescrashed on non-dict inputVariantClinicalSignificanceStatement.validate_tier_evidence_linesruns in pydanticmode="before"and calledvalues.get(...)unconditionally, so validating from a non-dict input (e.g.model_validate("propositions.json#/1")) raisedAttributeErrorinstead of a normal validation error. Added anisinstance(values, dict)passthrough guard, matching the existing nested type guards in the same validator.Verification
ruff/ruff-formatpre-commit hooks pass on both files.Nonepass 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