Skip to content

Genomic colocation: declare Short Variant as input, search, and category node - #220

Merged
jbrestel merged 7 commits into
masterfrom
feat/variant-colocation
Aug 17, 2026
Merged

Genomic colocation: declare Short Variant as input, search, and category node#220
jbrestel merged 7 commits into
masterfrom
feat/variant-colocation

Conversation

@jbrestel

Copy link
Copy Markdown
Member

What this is

The model half of adding Short Variant to the Genomic Colocation combine step.

Depends on VEuPathDB/ApiCommonWebService#22 — merge that one first. This PR declares a search whose plugin behaviour only exists there: without it, VariantsBySpanLogic resolves to a plugin that throws for VariantRecordClass. The two branches share the name feat/variant-colocation.

That companion PR is also substantially larger than its title suggests: it repairs five Oracle-isms that made genomic colocation entirely non-functional on PostgreSQL — not just for the new record type. Read it first for context.

Changes

spanParams.xml — adds VariantRecordClass to both span_a and span_b, the answerParams enumerating which record classes may be colocation inputs.

This is two lines and it is the whole feature in one direction. Without it a Short Variant result cannot be offered as an input at all, so variants → genes is impossible — the search would appear to work one way and be silently unavailable the other. An earlier draft of this design omitted it entirely.

spanQuestions.xml — adds VariantsBySpanLogic, replacing the commented-out SnpsBySpanLogic and SnpsChipsBySpanLogic blocks (plus an orphaned banner comment). No UI change is needed: ApiBinaryOperations.tsx matches colocation searches by convention, searchName.endsWith('BySpanLogic').

Four deliberate choices, flagged so they don't read as oversights:

  • includeProjects is copied from the VariantRecordClasses recordClassSet, so the search exists exactly where the record does.
  • sorting mirrors the record's own default (chromosome_order_num asc, location asc). Not variant_location asc — that is a textAttribute and not a safe sort key.
  • The three dynamic columns are named in attributesList as well as declared in dynamicAttributes, or they do not render.
  • No CDS or gene dynamic attributes. The old SNP question carried linkedGeneId and position_in_protein; both dropped on purpose.

individuals.txt — replaces the two SNP BySpanLogic ontology entries with one for VariantsBySpanLogic. Needed because wb model does not regenerate the category OWL; without it the search exists but is uncategorised, with no error anywhere. Note the existing quirk, mirrored rather than fixed: every BySpanLogic entry sits under the DynSpanRecordClass parent regardless of its own record type.

IsolatesBySpanLogic and OrfsBySpanLogic stay commented out.

docs/superpowers/specs/2026-08-15-variant-genomic-colocation-design.md — the design record. Its §6 is written for reviewers and covers the full Oracle-ism table plus the one decision in the companion PR that most deserves pushback: which schema owns the span temp tables. getDefaultSchema() returns the login schema on Oracle and a hardcoded public on PostgreSQL, while an unqualified CREATE TABLE follows search_path — so create and drop diverged, and a correct colocation surfaced as an error while leaking a table per run. The chosen fix and two rejected alternatives are documented there.

The doc also corrects its own earlier claim that the damage was two lines, and supersedes a prior draft that proposed a model-declared span-source contract (rejected: unvalidated one-row-per-record promise that Transcript and DynSpan could not use anyway).

Build note

Requires wb full — Java changed in ApiCommonWebService and individuals.txt changed here, so neither wb model nor wb ontology alone is sufficient.

Test plan

Verified live on a PlasmoDB dev instance against genomicsdb_071n:

  • VariantsBySpanLogic registered on record type variation (/service/record-types/variation)
  • Present in the category tree as a sibling of GenesBySpanLogic and DynSpansBySpanLogic
  • Gene → Variant
  • Variant → Gene (the direction the span_a/span_b change enables)
  • Variant → Genomic Span
  • Gene → Gene unaffected, including a 10001 bp upstream offset
  • Strand selector on a variant comparison returns results rather than zeroing out
  • UniDB not testedRecordsBySpanLogic declares wsColumn project_id unconditionally while VariantRecordClass excludes it from the PK there. Pre-existing and shared with DynSpanRecordClass; the shared query is deliberately untouched.

🤖 Generated with Claude Code

jbrestel and others added 6 commits August 15, 2026 11:59
Adds Short Variant to the Genomic Colocation combine step (both directions)
and repairs the Oracle->Postgres damage that limits colocation to genes<->genes.

Supersedes the 2026-08-13 draft, which rested on two claims that do not hold:
the Oracle syntax is a two-line fix rather than a design driver, and the
is_top_level/PAR argument attaches to the Transcript span source, not to an
unreachable FeatureLocation fallback.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
14 tasks, TDD against the WSFPlugin JUnit suite (which runs on the remote --
maven cannot resolve the parent POM locally).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Corrects this document's own claim that the Oracle->Postgres damage was two
lines. Live QA found two more, both in paths shared by every colocation, so
the feature was entirely non-functional on Postgres rather than gene-only.

Flags the create/drop schema choice for reviewer attention: the diff shows
only `schema` -> `null`, which hides a platform-semantics decision and two
rejected alternatives.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

@steve-fischer-200 steve-fischer-200 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see comment about include/exclude

also the superpowers files seem like a lot of noise

Comment thread Model/lib/wdk/model/questions/params/spanParams.xml
Same wrong method name as the code comments; caught in review on
ApiCommonWebService#22.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.

3 participants