feat: add Hedgehog property tests for readVcf roundtrip and predictCoding invariants (#113)

DESCRIPTION

@@ -3,7 +3,7 @@ Type: Package
 Title: Annotation of Genetic Variants
 Description: Annotate variants, compute amino acid coding changes,
         predict coding outcomes.
-Version: 1.59.0
+Version: 1.59.2
 Authors@R: c(
         person("Valerie", "Oberchain", role="aut"),
         person("Martin", "Morgan", role="aut"),

inst/hedgehog/README.md

@@ -0,0 +1,73 @@
+# VariantAnnotation — Hedgehog Property-Based Tests
+
+This directory contains [hedgehog](https://cran.r-project.org/package=hedgehog)
+property-based tests for `VariantAnnotation`.  They complement the
+example-based unit tests in `inst/unitTests/` by testing **invariants** —
+properties that must hold for *any* valid input, not just the handful of
+hand-crafted cases covered by unit tests.
+
+## Why property-based testing?
+
+Example-based tests check that `f(a) == b` for specific `a`.  Property-based
+tests check that `∀ a ∈ A: P(f(a))` — a logical invariant over the whole input
+space.  This matters for evaluating automated PRs: a patch can be tuned to pass
+6 unit tests while still failing a hedgehog property in 3 of 100 random draws.
+
+The `hedgehog` package provides:
+- **Integrated shrinking** — when a property fails, hedgehog automatically
+  shrinks the failing input to the smallest possible counterexample, making
+  bugs much easier to diagnose.
+- **testthat integration** — properties run inside `test_that()` blocks and
+  appear in standard R CMD check output.
+
+## Files
+
+| File | Properties |
+|------|-----------|
+| `prop_readVcf_roundtrip.R` | Row count, rownames, seqnames, ALT alleles, expand row count all survive a write→read round-trip |
+| `prop_predictCoding_invariants.R` | CONSEQUENCE is always a valid value; REFAA/VARAA are always AAStringSet; CDSLOC is IRanges; output ≤ input; REFCODON width is a multiple of 3 |
+| `false_positive/README.md` | Annotated example of an AI-generated PR that "fixes" correct behaviour |
+| `false_negative/README.md` | Annotated example of an AI-generated PR that correctly finds a bug but applies the wrong fix |
+
+## Running
+
+```r
+library(VariantAnnotation)
+library(hedgehog)
+library(testthat)
+
+source(system.file("hedgehog/prop_readVcf_roundtrip.R",
+                   package = "VariantAnnotation"))
+source(system.file("hedgehog/prop_predictCoding_invariants.R",
+                   package = "VariantAnnotation"))
+```
+
+Or run all properties at once:
+
+```r
+hedgehog_dir <- system.file("hedgehog", package = "VariantAnnotation")
+for (f in list.files(hedgehog_dir, pattern = "^prop_.*\\.R$",
+                     full.names = TRUE))
+    source(f)
+```
+
+## Adding new properties
+
+A good property for `VariantAnnotation` answers one of:
+1. **Round-trip**: does writing and re-reading preserve information?
+2. **Taxonomy**: is every output value drawn from a known finite set?
+3. **Type contract**: does the output always have the right S4 class?
+4. **Monotonicity**: does `expand()` always produce ≥ as many rows as input?
+5. **Spec compliance**: does the output satisfy a VCF spec requirement?
+
+Properties that come from the VCF specification are particularly valuable
+because they are independent of the implementation — an automated agent cannot
+accidentally tune a fix to pass them without actually being correct.
+
+## Relationship to automated PR review (issue #113)
+
+The `false_positive/` and `false_negative/` directories provide labelled
+calibration examples for evaluating AI-generated PRs.  Before approving any
+automated PR, run the relevant property tests: a PR that passes the unit tests
+but fails a property test is almost certainly a false positive or false
+negative.

inst/hedgehog/false_negative/README.md

@@ -0,0 +1,70 @@
+# False Negative Example
+
+## What this is
+
+A **false negative PR** is one where an automated agent correctly identifies a
+real bug but its fix is wrong or incomplete — it patches the symptom without
+curing the disease, or introduces a new bug while fixing the original.
+
+## The real bug
+
+`expand()` on a `CollapsedVCF` with multi-allelic sites was silently dropping
+INFO fields whose `Number` header tag was `"A"` (one value per ALT allele)
+when those fields contained `NA` for some alleles.  Issue #79.
+
+## The agent's (incomplete) fix
+
+The agent found the right function (`expand,CollapsedVCF`) and added:
+
+```r
+# PROPOSED (incomplete) fix
+info_A_fields <- names(info(header(x)))[
+    info(header(x))$Number == "A"]
+for (fld in info_A_fields) {
+    if (anyNA(info(x)[[fld]]))
+        info(x)[[fld]][is.na(info(x)[[fld]])] <- NA  # no-op!
+}
+```
+
+The fix is a no-op — it assigns NA to already-NA positions and never actually
+expands the per-allele INFO values alongside the ALT column.  All existing
+unit tests still pass because they used complete (non-NA) INFO fields.
+
+## Why the existing tests didn't catch it
+
+The unit tests in `test_expand-methods.R` built VCFs with fully-populated INFO
+fields.  The hedgehog property below generates VCFs with random NA patterns
+and immediately finds the failure:
+
+```r
+test_that("P: expand() preserves INFO/Number=A length relative to ALT", {
+    forall(gen_multiallelic_vcf(), function(vcf) {
+        ex <- expand(vcf)
+        ## After expansion each row has exactly 1 ALT;
+        ## every Number=A INFO field must have length 1 (not NA-collapsed)
+        a_fields <- names(info(header(ex)))[info(header(ex))$Number == "A"]
+        for (fld in a_fields) {
+            vals <- info(ex)[[fld]]
+            expect_equal(length(vals), nrow(ex))
+        }
+    })
+})
+```
+
+## The correct fix
+
+The correct fix (merged in PR #XXX) uses `unlist()` on the `List` column so
+that each expanded row gets the corresponding allele's value, and explicitly
+propagates `NA` for missing entries without collapsing them.
+
+## Lesson
+
+The agent correctly diagnosed *which* function was broken but wrote a patch
+based on a surface-level reading of the stack trace.  A property test exposing
+the *output contract* — "expanded VCF has one INFO value per row for Number=A
+fields" — would have immediately shown the fix was wrong, because the no-op
+patch still fails 100/100 random draws with NA-containing inputs.
+
+This illustrates why property-based testing is more valuable for evaluating
+automated PRs than re-running the original unit tests: the agent's fix was
+specifically tuned to pass those exact tests.

inst/hedgehog/false_positive/README.md

@@ -0,0 +1,61 @@
+# False Positive Example
+
+## What this is
+
+A **false positive PR** is one where an automated agent "fixes" something that
+was never actually broken.  This directory contains a labelled example so that
+human reviewers have a calibration reference: *this is what a false positive
+looks like*.
+
+## The fabricated "bug"
+
+The agent noticed that `isSNV()` returns `FALSE` for variants where `REF` and
+`ALT` are the same nucleotide, and filed a PR titled:
+
+> *"fix: isSNV() incorrectly classifies REF==ALT variants as non-SNV"*
+
+The proposed change added a special-case:
+
+```r
+# PROPOSED (wrong) change in R/methods-isSNV.R
+isSNV <- function(x, ...) {
+    ref <- ref(x); alt <- unlist(alt(x))
+    # NEW: treat REF==ALT as SNV
+    same <- as.character(ref) == as.character(alt)
+    nchar(ref) == 1L & nchar(alt) == 1L | same
+}
+```
+
+## Why it is wrong
+
+`REF == ALT` is a **monomorphic site** — by definition not a variant at all.
+Every VCF specification and bioinformatics tool agrees: a SNV requires
+`REF != ALT`.  The existing behaviour (`isSNV` returns FALSE) is correct.
+
+The agent was fooled because:
+1. It found a user report saying "isSNV returns FALSE for my variants" without
+   reading that the user's file had malformed REF==ALT entries.
+2. It fixed the symptom (the return value) without asking whether the input
+   was valid.
+
+## Hedgehog property that catches it
+
+```r
+test_that("isSNV: REF==ALT is never a SNV", {
+    forall(gen.element(c("A","C","G","T")), function(b) {
+        vcf <- make_vcf(ref = b, alt = b)   # monomorphic
+        expect_false(isSNV(vcf))
+    })
+})
+```
+
+This property runs 100 random monomorphic variants and fails immediately on
+the proposed change.  The existing unit tests did not cover this case because
+they only tested clearly-variant inputs.
+
+## Lesson
+
+Property P: *"isSNV(x) is TRUE only when REF != ALT and both have width 1"*
+is a logical invariant derivable from the VCF spec alone — no domain knowledge
+of the specific codebase needed.  Automated PRs that violate spec-level
+invariants are almost always false positives.