From 3fdc46d4c9c2a4eedb31c9dd70952cff6689ed2b Mon Sep 17 00:00:00 2001 From: John Brestelli Date: Fri, 31 Jul 2026 11:38:06 -0400 Subject: [PATCH 01/42] Add design spec for internal strain genomic segment record Introduces the design for a new internal `strain-genomic-segment` WDK record class that translates reference-coordinate genomic segments into isolate/strain coordinates using apidb.indel, terminating at a BED reporter. Key decisions recorded: - Primary key carries reference coordinates plus strain; strain coordinates are derived in an attribute query, preserving provenance and picking up indel reloads without recomputation. - Strain vocabulary comes from apidb.indel joined through study.protocolappnode (there is no strain column on apidb.indel); scoped per organism. - Queries against partitioned webready.*_p tables must constrain org_abbrev. - BED chrom must equal _, the dnaseq consensus FASTA index key; the name column is free and becomes the output defline. Co-Authored-By: Claude Opus 5 --- ...2026-07-31-strain-segment-record-design.md | 306 ++++++++++++++++++ 1 file changed, 306 insertions(+) create mode 100644 docs/superpowers/specs/2026-07-31-strain-segment-record-design.md diff --git a/docs/superpowers/specs/2026-07-31-strain-segment-record-design.md b/docs/superpowers/specs/2026-07-31-strain-segment-record-design.md new file mode 100644 index 000000000..45f1ffadd --- /dev/null +++ b/docs/superpowers/specs/2026-07-31-strain-segment-record-design.md @@ -0,0 +1,306 @@ +# Strain genomic segment record — design + +**Date:** 2026-07-31 +**Status:** draft (design agreed in session; not yet reviewed) +**Scope:** WDK record class, one search, attribute query, and BED reporter for a new +**internal** `strain-genomic-segment` record. **FASTA/seqret wiring is out of scope** — +the pipeline deliberately stops at the BED reporter (§8). +**Implementation target:** `ApiCommonModel` + `ApiCommonWebsite`, branch +`strain-segment-record` (created off `master`), fungidb checkout. + +**Path conventions:** paths are relative to their repository root. `Model/lib/wdk/...` +is in `ApiCommonModel` (this repo); `Model/src/main/java/org/apidb/apicommon/model/report/...` +is in `ApiCommonWebsite`; `bin/` and `modules/` are in `dnaseq-nextflow`; +`src/main/java/org/veupathdb/service/sr/...` is in `service-sequence-retrieval`. + +## 1. Purpose + +Deliver a FASTA download of genomic segments expressed in **isolate/strain +coordinates**, given segment coordinates in **reference** coordinates as input. + +Strain consensus sequences differ in length from the reference — `build_consensus` in +`dnaseq-nextflow/bin/makeConsensusFastaFromVcfAndBed.py` emits the called allele for +homozygous indels — so a reference interval does not address the same bases in a strain +contig. `apidb.indel` records the per-event shifts needed to translate between them. + +The record class exists **only** to carry this business logic from user-selected strain + +reference location to a BED feature. It is not user-facing, has no record page, and +supports no saved strategies. + +### Non-goals + +- No record page, no summary view, no tables, no `fullRecord`/`attributesTabular` + reporters. A record-page request for this class should fail, not render empty. +- No saved strategies. This removed the only argument for baking strain coordinates + into the primary key (§3). +- No coding/protein coordinates. `apidb.indel` is genomic; `codingIndels.db` is a + separate upstream artifact and a separate problem. + +## 2. Data sources + +Measured 2026-07-31 against `genomicsdb_rebuild01` (ssh tunnel, `localhost:5439`). + +### `apidb.indel` — 43,585,584 rows + +``` +indel_id, protocol_app_node_id, na_sequence_id, location, shift (+ GUS housekeeping) +PK indel_id; ix0 (na_sequence_id, indel_id); ix1 (protocol_app_node_id, indel_id) +FK -> dots.nasequenceimp, study.protocolappnode +``` + +6,249 distinct `protocol_app_node_id`, 20,823 distinct `na_sequence_id`. + +**There is no strain column.** Upstream there is — `makeGenomicIndelDb` in +`dnaseq-nextflow/modules/mergeExperiments.nf` builds +`genomic_indels(strain, sequence_id, position, shift)` — but the GUS load normalizes +strain into `study.protocolappnode`, so on the website side strain identity is one join +away. This is the single most misleading thing about the table and the reason §4.1 exists. + +`shift` is a **signed per-event delta**, never zero: range −101..+80, with 21,945,521 +negative and 21,640,063 positive rows. The strain offset at a reference position is +therefore a *prefix sum*, partitioned by `(protocol_app_node_id, na_sequence_id)`. + +### `study.protocolappnode` — strain vocabulary + +Strain name is `name` with the suffix `_Indel` removed: `A0003_Indel`, `S7_Indel`, +`A17-48H-7_Indel`, `X10462-P1C9_Indel`. + +Verified properties of the 6,119 distinct names appearing in `apidb.indel`: + +| Property | Result | Consequence | +|---|---|---| +| contain `:` | **0** | the colon-delimited PK grammar (§3) is unambiguous | +| end in `_Indel` | **6,119 / 6,119** | the suffix strip is uniform; no special cases | +| contain any other `_` | **0** | `_` concatenation is unambiguous | +| map to >1 organism | **126** (256 nodes) | strain name is **not** globally unique — see §5.2 | +| `(name, na_sequence_id)` -> >1 node | **0** | sequence-scoped resolution is unambiguous *today* | + +That last row is a **data-dependent invariant with no constraint enforcing it**. §5.2 +specifies a query shape that turns a future violation into an error rather than silent +corruption. + +### `webready.genomicseqattributes_p` — reference sequences + +`Partition key: LIST (org_abbrev)`. Every unique index is `org_abbrev`-prefixed +(`seqattr_source_id`, `seqattr_naseqid`, `pk_seqattr_`, `seqattr_taxsrc_id`). + +> **Every query against a `webready.*_p` table must constrain `org_abbrev`.** The column +> is `org_abbrev`, not `organism_abbrev`. Omitting it scans all partitions (831 on +> `organismabbreviation_p`) and uses no index. + +`organism -> org_abbrev` comes from `webready.organismabbreviation_p` +(`organism`, `org_abbrev`, `project_id`, `sanitized_org_abbrev`, `name_for_filenames`). + +Note: `apidbtuning.organismattributes` and `apidbtuning.GenomicSeqAttributes` — both +joined by DynSpan's `Bfmv` query — **do not exist in this database**. Only the +`webready.*_p` forms do. New queries target `webready.*_p` exclusively. + +### Strain consensus FASTA (for §7 context only) + +`makeConsensusFastaFromVcfAndBed.py:233` writes deflines as `_`, i.e. +**`_`**. `checkUniqueIds.sh` hard-fails (exit 125) on any duplicate +defline across the merged file, so it is **one merged multi-strain FASTA**, not one file +per strain — meaning one seqret sequence type suffices whenever §8 is picked up. + +## 3. Identity + +``` +source_id = ::-: + A0003:AACB03000001:100-200:f +``` + +Primary key columns: `source_id`, plus `project_id` with `excludeProjects="UniDB"` and +UniDB-specific `` variants, mirroring the existing fork at +`dynSpanAttributeQueries.xml:16-33`. **UniDB is included** for this record and is to be +added to the existing genomic-segment `includeProjects` lists where absent. + +Coordinates in the PK are **reference** coordinates — the input, not the output. Strain +coordinates are derived in the attribute query (§5.2). Rationale: + +- Provenance: the defline can report both what was requested and what was returned. +- A reload of the dnaseq indel data is picked up automatically; nothing recomputes stale. +- `(strain, refSeq)` resolves to exactly one `protocol_app_node_id` (§2), so + strain-in-the-PK is sufficient to validate strain against the indel vocabulary, which + is the stated requirement. + +### 3.1 Parse the grammar once + +DynSpan implements its ID grammar **five** times — SQL `regexp_substr` at +`dynSpanAttributeQueries.xml:66-71` and `spanQueries.xml:51,62`, SQL `CONCAT` +construction at `spanQueries.xml:120,197,275,396`, a Java regex at +`DynSpanFeatureProvider.java:18`, and construction at `GffSpanDatasetParser.java:66`. +The SQL and Java parsers **disagree**: SQL takes the first colon token as the sequence +ID, the Java regex is greedy (`^(.*):(\d+)-(\d+):(f|r)$`) and takes the last two tokens +as range and strand. A sequence ID containing a colon parses differently on each side. + +This spec adds **one** Java class, `StrainSegmentId`, with `parse`/`format`, used by the +feature provider. On the SQL side use `split_part(source_id, ':', N)`, not positional +`regexp_substr` — it does not silently renumber fields when one is empty. Strain names +contain `-` (`A17-48H-7`), which is safe only because the range is its own colon field; +the range must never be parsed out of the whole string positionally. + +## 4. Where DynSpan gets validation wrong + +Recorded because the new search must not repeat it. + +| Location | Behaviour | +|---|---| +| `spanQueries.xml:10-22` (`DynSpansBySegIds`) | webservice-only; **zero** validation. Any string becomes a record. | +| `spanQueries.xml:48-67` (`DynSpansBySourceId`) | only existence check anywhere; **silently drops** bad rows. Does not check organism membership or sequence length. Its organism/chromosome params appear in no `WHERE` clause — they are form scaffolding. | +| `dynSpanAttributeQueries.xml:74-75` (`Bfmv`) | `LEFT JOIN` seq attrs then `INNER JOIN` organism attrs, so an unknown sequence yields a null organism that the inner join deletes. Attributes vanish; no error. | +| `DynSpanFeatureProvider.java:46-48` | the **only** place that throws — at download time, after the search succeeded. | + +## 5. Model changes (`ApiCommonModel`) + +### 5.1 Search: `StrainSegmentId.StrainSegmentsByRefSegment` + +Input is a **single reference location**, modeled on `DynSpansByLocation` +(`spanQueries.xml:266`), not a `datasetParam` — so `SpanParams.span_id` and its +`recordClassRef` to DynSpan are not involved. + +Params: `organism` (single pick), `strain` (enum, **dependent on organism**), +`sequenceId`, `start`, `end`, `strand`. + +The organism param's **internal value is `org_abbrev`**, so the partition key is +available directly to every query without an extra lookup. Precedent: +`organismParams.organism_span` is compared straight against `org_abbrev` columns at +`spanQueries.xml:203-205`. The display organism name, when needed for a defline or a +join, comes from `webready.organismabbreviation_p` keyed on the same `org_abbrev`. + +Three gates, all as joins/filters so bad input yields **zero records** rather than a +broken download: + +1. `strain` is in the vocabulary for the chosen organism (§5.2 query); +2. `refSeq` belongs to the chosen organism — join `webready.genomicseqattributes_p` + constraining `org_abbrev` to the chosen organism, which is both the partition key and + the organism-membership check that `DynSpansBySourceId` omits; +3. `1 <= refStart <= refEnd <= gsa.length`. + +### 5.2 Strain vocabulary (dependent param) + +```sql +SELECT DISTINCT regexp_replace(pan.name, '_Indel$', '') AS strain +FROM apidb.indel i + JOIN study.protocolappnode pan ON pan.protocol_app_node_id = i.protocol_app_node_id + JOIN webready.genomicseqattributes_p gsa ON gsa.na_sequence_id = i.na_sequence_id +WHERE gsa.org_abbrev = $$organism$$ -- partition key; param's internal value +``` + +(`$$organism$$` is the `org_abbrev` internal value per §5.1, so this both scopes the +vocabulary to the chosen organism and hits the partition.) + +This is the controlled vocabulary required by the brief: strain names come from +`apidb.indel`, scoped per organism. No tuning table exists for it. + +### 5.3 Attribute query: `StrainSegmentAttributes.Coords` + +Emits `strain_seq_id`, `strain_start`, `strain_end`, `strain_length`, `organism`. + +`strain_seq_id` is `strain || '_' || refSeq` — pure concatenation, matching the FASTA +key (§2). Offsets in one index-assisted pass per `(strain, sequence)`, bounded by +`location <= ref_end`, using conditional aggregation rather than two correlated +subqueries: + +```sql +SUM(CASE WHEN i.location < seg.ref_start THEN i.shift ELSE 0 END) AS offset_start, +SUM(CASE WHEN i.location <= seg.ref_end THEN i.shift ELSE 0 END) AS offset_end +``` + +**Resolve `protocol_app_node_id` via `(name, na_sequence_id)` and `GROUP BY` the +resolved node — never join on name alone.** 126 strain names map to more than one +organism (§2). Grouping by node makes a future collision produce two rows per primary +key, which WDK rejects with an error. Joining on name alone would sum two strains' +shifts into one plausible-looking wrong answer with nothing to detect it. + +**Boundary rule (to be QA'd, per §9):** an indel exactly at `refStart` lies inside the +segment, so it shifts the end but not the start — hence `<` for start, `<=` for end. An +off-by-one here is silent and yields sequence that looks correct. + +## 6. Record class (`ApiCommonModel`) + +`StrainSegmentRecordClasses.StrainSegmentRecordClass`, +`urlName="strain-genomic-segment"`, `doNotTest="true"`. + +Reporters: **`bed` only** (see §8). Attributes: exactly those §5.3 emits, plus the +`idAttribute`. No tables, no summary view, no text attributes for display. + +## 7. BED reporter (`ApiCommonWebsite`) + +`BedStrainSegmentReporter extends BedReporter` + `StrainSegmentFeatureProvider +implements BedFeatureProvider`, following `BedGenomicSequenceReporter` / +`GenomicSequenceFeatureProvider` — the existing precedent for a provider that computes +coordinates from **attributes** rather than from the PK. + +`getRequiredAttributeNames()` = `{strain_seq_id, strain_start, strain_end, organism}`. +Strain is parsed from the PK, so it needs no attribute. + +The two BED columns have very different constraints: + +| Column | Constraint | +|---|---| +| `chrom` | **Hard.** Must equal `_` — the strain FASTA index key. Lookup fails otherwise. | +| `name` | **Free.** Under `deflineFormat=QUERYONLY`, `Deflines.deflineForFeature` emits `'>' + feature.getName()` verbatim, so this becomes the output defline. Ours to design. | + +So `chrom = strain_seq_id`, and the `name` column carries provenance via +`DeflineBuilder`, honouring `RequestedDeflineFields` as `DynSpanFeatureProvider` does +(`DynSpanFeatureProvider.java:57-72`): + +``` +>A0003:AACB03000001:100-200:f | Aspergillus fumigatus Af293 | A0003 | ref 100 to 200 | strain 143 to 241 | segment_length=99 +``` + +Carrying both coordinate systems is the payoff for choosing reference coordinates in +the PK (§3). + +## 8. Deferred: seqret wiring + +Sequence types in `service-sequence-retrieval` are pure configuration — +`ReferenceDAOFactory.init()` reads `ALL_REFERENCE_SEQUENCE_NAMES` plus per-name +`_FASTA_FILE`, `_INDEX_FILE`, `_IS_STRANDED`, `_MAX_SEQUENCES_PER_REQUEST`, +`_MAX_TOTAL_BASES_PER_REQUEST`. **No service code change is needed.** + +When picked up, the work is: add a `SequenceType` enum value and one switch arm in +`SequenceReporter.getSequenceTypeByRecordClassFullName` (`SequenceReporter.java:94-113`) +— the single hard-coded record-class-to-FASTA seam — register the `sequence` reporter on +the record class, and add the env vars. `SequenceType.name()` goes straight into the +request URL and `ReferenceDAOFactory.get()` lowercases, so the enum name must equal the +configured sequence name. + +## 9. Verification + +**Development target is fungidb** (`~/workspaces/fungidb`, branch +`strain-segment-record`, sync up). **Not giardiadb:** it has 6,902 Giardia sequences +across 14 organisms and **zero** `apidb.indel` rows, so nothing there exercises this code. + +QA organism: *Aspergillus fumigatus* Af293 — 1,117 strains, 5,085,377 indels. Other +dense options: *Cryptococcus neoformans* H99 (875), *Candida auris* B8441 (502), +*Cryptococcus deuterogattii* R265 (293). + +1. **SQL as WDK runs it** — `wdkQuery -model FungiDB -query + StrainSegmentAttributes.Coords -showQuery` (and `-showParams`). Renders post-injection + SQL without executing. Requires a prior `wb model`. +2. **Execute** the rendered SQL read-only in psql against the tunnelled genomicsdb. + `%%PARTITION_KEYS%%` does not apply — this is not a partitioned gene-table query. +3. **Registration** — `/service/record-types/strain-genomic-segment` lists the class and + its searches. Fetch from an authenticated app page (`javascript_tool`), since a raw + curl 307s to autologin. +4. **Invisibility** — the search must be **absent** from `/service/ontologies/Categories`. + That endpoint is not project-filtered, so absence there is the meaningful check. +5. **Shift correctness** — the boundary rule in §5.3. Pick a strain and sequence with + known indels, compute the expected prefix sum directly in psql, compare against + `strain_start`/`strain_end`. +6. **Free signal for §5.3 once §8 lands:** seqret validates the feature against the + strain contig's indexed length and *fails open*, appending + `" | error_code=NOT_REQUESTED_LENGTH"` to the defline when `end > indexLength` + (`Deflines.java`). A correct implementation never produces it. It is silent by + design — nothing errors — so it only helps if deflines are actually read. + +## 10. Risks and open items + +| Item | Status | +|---|---| +| `<`/`<=` boundary at `refStart` | **open** — ships as specified, QA per §9.5 | +| Ontology absence hides the search | **unverified assumption** — the `DynSpansBySegIds` precedent was read but `Model/lib/wdk/ontology/individuals.txt` was never opened. Confirm before relying on it. | +| `(name, na_sequence_id)` uniqueness | true today, unconstrained; §5.3 grouping converts a violation to an error | +| Prefix-sum cost | heaviest strain/sequence pair carries ~115k events; the `location <= ref_end` bound plus `ix0`/`ix1` should hold, but measure on Af293 | +| Categorization rebuild | any change touching categorization needs `wb ontology`, not `wb model` — `wb ontology` is a superset and reloads the webapp | From 91c5989fabbbcc478c451b9bc99e9c64565dd3e0 Mon Sep 17 00:00:00 2001 From: John Brestelli Date: Fri, 31 Jul 2026 11:42:02 -0400 Subject: [PATCH 02/42] Resolve the ontology-visibility question in the strain segment spec The spec flagged "ontology absence hides the search" as an unverified assumption. Verified against Model/lib/wdk/ontology/individuals.txt and recorded the result as a new section 6.1. Two mechanisms exist for hiding a search. Omitting the row is correct here, matching DynSpansBySegIds and DynSpansByLocation, which are both absent from individuals.txt while remaining addressable through the service API. The "internal webservice" scope combination is used only by injected per-dataset searches, which need ontology presence for categorization. Consequence: this work adds no ontology node, so wb model suffices and wb ontology is not required. Co-Authored-By: Claude Opus 5 --- ...2026-07-31-strain-segment-record-design.md | 46 ++++++++++++++++++- 1 file changed, 44 insertions(+), 2 deletions(-) diff --git a/docs/superpowers/specs/2026-07-31-strain-segment-record-design.md b/docs/superpowers/specs/2026-07-31-strain-segment-record-design.md index 45f1ffadd..e1b323710 100644 --- a/docs/superpowers/specs/2026-07-31-strain-segment-record-design.md +++ b/docs/superpowers/specs/2026-07-31-strain-segment-record-design.md @@ -224,6 +224,48 @@ off-by-one here is silent and yields sequence that looks correct. Reporters: **`bed` only** (see §8). Attributes: exactly those §5.3 emits, plus the `idAttribute`. No tables, no summary view, no text attributes for display. +Register the new files in `Model/lib/wdk/apiCommonModel.xml`, alongside the existing +span imports at lines 419-423. + +### 6.1 Keeping it non-user-facing + +`Model/lib/wdk/ontology/individuals.txt` is a 14-column TSV: + +| col | meaning | +|---|---| +| 1 | full node name | +| 4 | `recordClassName` | +| 5 | `targetType` (`search` / `table` / `attribute` / `dataset`) | +| 6 | `name` (e.g. `SpanQuestions.DynSpansBySourceId`) | +| 12-14 | three `scope` columns | + +Scope combinations across the 202 `targetType=search` rows, measured 2026-07-31: + +| scopes | count | +|---|---| +| `menu` `webservice` | 95 | +| `internal` `webservice` | 42 | +| *(none)* | 27 | +| `webservice` | 17 | +| `menu` | 15 | +| `internal` | 6 | + +**Decision: add no row at all for the new search.** Two mechanisms could hide it, and +omission is the right one here: + +- **Omission** — `DynSpansBySegIds` (commented "SegIds only WEBSERVICES", + `spanQueries.xml:7`) and `DynSpansByLocation` are both absent from `individuals.txt` + and the model builds. A question stays addressable by name through the service API + regardless of ontology presence. This is the precedent for a *hand-written* + webservice-only span search — exactly our case. +- **`internal` + `webservice`** — all 42 users are *injected per-dataset* searches, which + need ontology presence for categorization to work. Not our situation. + +Consequence for verification: the search must be **absent** from +`/service/ontologies/Categories` (§9.4). Also, since no ontology node is added, this work +does **not** touch categorization — so `wb model` suffices and `wb ontology` is not +required. (It would be required if a row were ever added.) + ## 7. BED reporter (`ApiCommonWebsite`) `BedStrainSegmentReporter extends BedReporter` + `StrainSegmentFeatureProvider @@ -300,7 +342,7 @@ dense options: *Cryptococcus neoformans* H99 (875), *Candida auris* B8441 (502), | Item | Status | |---|---| | `<`/`<=` boundary at `refStart` | **open** — ships as specified, QA per §9.5 | -| Ontology absence hides the search | **unverified assumption** — the `DynSpansBySegIds` precedent was read but `Model/lib/wdk/ontology/individuals.txt` was never opened. Confirm before relying on it. | +| Ontology absence hides the search | **verified** 2026-07-31 — see §6.1 | | `(name, na_sequence_id)` uniqueness | true today, unconstrained; §5.3 grouping converts a violation to an error | | Prefix-sum cost | heaviest strain/sequence pair carries ~115k events; the `location <= ref_end` bound plus `ix0`/`ix1` should hold, but measure on Af293 | -| Categorization rebuild | any change touching categorization needs `wb ontology`, not `wb model` — `wb ontology` is a superset and reloads the webapp | +| Categorization rebuild | not applicable — §6.1 adds no ontology node, so `wb model` suffices. If a row is ever added, it becomes `wb ontology` (a superset of `wb model`). | From d33a8f59a1ad2a2891768f552945593fddc7b782 Mon Sep 17 00:00:00 2001 From: John Brestelli Date: Fri, 31 Jul 2026 11:53:07 -0400 Subject: [PATCH 03/42] Add implementation plan for the strain genomic segment record Eight tasks, each independently committable, covering the grammar class with unit tests, the strain vocabulary param, the validating ID query, the coordinate-conversion attribute query, the record class and model imports, the BED reporter, end-to-end verification, and a UniDB scope fix on the existing eQTL segment search. Two findings from writing it that shaped the tasks: - The attribute query cannot use webready.*_p at all. Attribute queries receive only primary key columns, so there is no org_abbrev to prune partitions with, and every index on those tables is org_abbrev-prefixed. There is also no unpartitioned GenomicSeqAttributes in this database, which is why DynSpan's Bfmv query cannot run here. The plan routes through dots.ExternalNaSequence instead, which is unpartitioned and indexed on source_id. - Tasks 2 through 4 land model XML before it is imported, so each commits safely without a working model. Task 5 adds the imports and is the first task that can fail a build. Co-Authored-By: Claude Opus 5 --- .../plans/2026-07-31-strain-segment-record.md | 1234 +++++++++++++++++ 1 file changed, 1234 insertions(+) create mode 100644 docs/superpowers/plans/2026-07-31-strain-segment-record.md diff --git a/docs/superpowers/plans/2026-07-31-strain-segment-record.md b/docs/superpowers/plans/2026-07-31-strain-segment-record.md new file mode 100644 index 000000000..6c3f4af4f --- /dev/null +++ b/docs/superpowers/plans/2026-07-31-strain-segment-record.md @@ -0,0 +1,1234 @@ +# Strain Genomic Segment Record Implementation Plan + +> **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. + +**Goal:** Add an internal `strain-genomic-segment` WDK record class that takes a single reference-coordinate genomic location plus a strain, and emits a BED feature in that strain's consensus-sequence coordinates. + +**Architecture:** The record's primary key carries *reference* coordinates plus the strain (`::-:`). One ID query validates strain, reference sequence, and range, returning zero rows if any gate fails. One attribute query converts reference to strain coordinates by prefix-summing `apidb.indel.shift`. A `BedFeatureProvider` reads those attributes and writes a BED line whose `chrom` is `_` — the key into the strain consensus FASTA. The pipeline deliberately stops at BED; seqret wiring is out of scope. + +**Tech Stack:** WDK model XML (`ApiCommonModel`), Java 11 + JUnit 4 (`ApiCommonWebsite/Model`), PostgreSQL 18, Maven, `wb` build wrapper. + +**Design spec:** `docs/superpowers/specs/2026-07-31-strain-segment-record-design.md` (this repo). Read it first — it records the measured data facts and the reasoning behind each decision. + +--- + +## Orientation for someone new to this codebase + +**Two repos, one branch name.** Model XML lives in `ApiCommonModel`; Java lives in `ApiCommonWebsite`. Both have a `strain-segment-record` branch off `master` in `~/workspaces/fungidb/`. Commit in whichever repo you touched. + +**Nothing builds locally.** `~/workspaces/fungidb` is synced by mutagen to `cedar:/var/www/jbrestel.fungidb.org/project_home`, and all builds and tests run on `cedar`. Edit locally, save, and mutagen carries the bytes within a second or two. Never build in the local checkout. + +**After any commit or branch switch, run `bash ~/workspaces/agentic-veupath-dev/bin/veup-git-sync.sh fungidb`.** Mutagen ignores `.git`, so the remote's refs go stale and its `git status` fills with modifications that are not real. This script reconciles refs only and never touches file contents. A local commit you have not pushed reports `UNPUSHED` and is skipped — that is expected and harmless here, since the build reads files, not git. + +**An XML file that no one imports is inert.** `Model/lib/wdk/apiCommonModel.xml` lists every model file. Until you add an ``, a new file is not parsed and cannot break the build. Tasks 2-4 exploit this: they land XML that is not yet imported, so each can be committed safely without a working model. Task 5 wires it in and is the first task that can fail a build. + +**Useful commands:** + +```bash +# build the model on the remote (also reloads the webapp) +bash ~/workspaces/agentic-veupath-dev/bin/veup-build.sh fungidb wb model + +# render a query's SQL exactly as WDK will run it, without executing +ssh cedar 'bash -lc "source /var/www/jbrestel.fungidb.org/etc/setenv && \ + wdkQuery -model FungiDB -query -showQuery"' + +# read-only SQL against the app database +psql -h localhost -p 5439 -d genomicsdb_rebuild01 -c "" + +# log delta around a page load +bash ~/workspaces/agentic-veupath-dev/bin/veup-logs.sh fungidb mark t1 +bash ~/workspaces/agentic-veupath-dev/bin/veup-logs.sh fungidb since t1 +``` + +`wb ontology` is **not** needed: this work adds no ontology node (spec §6.1). `wb model` suffices. + +--- + +## File structure + +**`ApiCommonModel`** — all paths under `Model/lib/wdk/`: + +| File | Responsibility | +|---|---| +| `model/questions/params/strainSegmentParams.xml` | *new* — the `strain` flat-vocab param and its organism-dependent vocabulary query | +| `model/questions/queries/strainSegmentQueries.xml` | *new* — the ID query; all four validation gates live here | +| `model/records/strainSegmentAttributeQueries.xml` | *new* — the reference-to-strain coordinate conversion | +| `model/records/strainSegmentRecord.xml` | *new* — record class, attributes, BED reporter registration | +| `model/questions/strainSegmentQuestions.xml` | *new* — the question that binds query to record class | +| `apiCommonModel.xml` | *modify* — five `` lines near the existing span imports at 419-423 | + +**`ApiCommonWebsite`** — all paths under `Model/src/`: + +| File | Responsibility | +|---|---| +| `main/java/org/apidb/apicommon/model/report/bed/util/StrainSegmentId.java` | *new* — the single parse/format authority for the PK grammar | +| `test/java/org/apidb/apicommon/model/report/bed/util/StrainSegmentIdTest.java` | *new* — unit tests for the grammar | +| `main/java/org/apidb/apicommon/model/report/bed/feature/StrainSegmentFeatureProvider.java` | *new* — record to BED fields | +| `main/java/org/apidb/apicommon/model/report/bed/BedStrainSegmentReporter.java` | *new* — three-line reporter wiring | + +One responsibility each. The grammar class is the only piece with no WDK dependencies, which is exactly why it is the only piece that gets real unit tests — everything else needs a live `RecordInstance` and is verified against the running site. + +--- + +## Task 1: The PK grammar class + +The `ApiCommonWebsite/Model` module has JUnit 4 on its classpath but no `src/test/java` tree yet. You are creating it. Standard Maven layout means Surefire picks it up with no POM change. + +**Files:** +- Create: `ApiCommonWebsite/Model/src/test/java/org/apidb/apicommon/model/report/bed/util/StrainSegmentIdTest.java` +- Create: `ApiCommonWebsite/Model/src/main/java/org/apidb/apicommon/model/report/bed/util/StrainSegmentId.java` + +- [ ] **Step 1: Write the failing test** + +Create `StrainSegmentIdTest.java`: + +```java +package org.apidb.apicommon.model.report.bed.util; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.fail; + +import org.junit.Test; + +public class StrainSegmentIdTest { + + @Test + public void parsesForwardStrandId() { + StrainSegmentId id = StrainSegmentId.parse("A0003:AACB03000001:100-200:f"); + assertEquals("A0003", id.getStrain()); + assertEquals("AACB03000001", id.getRefSeq()); + assertEquals(100, id.getRefStart()); + assertEquals(200, id.getRefEnd()); + assertEquals(StrandDirection.forward, id.getStrand()); + } + + @Test + public void parsesReverseStrandId() { + StrainSegmentId id = StrainSegmentId.parse("S7:AACB03000001:1-50:r"); + assertEquals("S7", id.getStrain()); + assertEquals(StrandDirection.reverse, id.getStrand()); + } + + // The BED chrom column must equal the dnaseq consensus FASTA defline, + // which makeConsensusFastaFromVcfAndBed.py writes as _. + @Test + public void strainSeqIdMatchesFastaKey() { + assertEquals("A0003_AACB03000001", + StrainSegmentId.parse("A0003:AACB03000001:100-200:f").getStrainSeqId()); + } + + // Real strain names contain hyphens; the range is a separate colon field so this is safe. + @Test + public void strainNameWithHyphensSurvives() { + StrainSegmentId id = StrainSegmentId.parse("X10462-P1C9:AACB03000001:1-50:r"); + assertEquals("X10462-P1C9", id.getStrain()); + assertEquals(1, id.getRefStart()); + assertEquals(50, id.getRefEnd()); + } + + @Test + public void formatRoundTripsBothStrands() { + String forward = "A17-48H-7:AACB03000001:5-9:f"; + String reverse = "A17-48H-7:AACB03000001:5-9:r"; + assertEquals(forward, StrainSegmentId.parse(forward).format()); + assertEquals(reverse, StrainSegmentId.parse(reverse).format()); + } + + // DynSpan's greedy "^(.*):" regex silently mis-parses these. We reject instead. + @Test + public void rejectsColonInSequenceIdRatherThanMisparsing() { + assertRejected("A0003:AAC:B03:100-200:f"); + } + + @Test + public void rejectsMissingStrand() { + assertRejected("A0003:AACB03000001:100-200"); + } + + @Test + public void rejectsMissingStrain() { + assertRejected("AACB03000001:100-200:f"); + } + + @Test + public void rejectsStartGreaterThanEnd() { + assertRejected("A0003:AACB03000001:200-100:f"); + } + + @Test + public void rejectsNonNumericCoordinates() { + assertRejected("A0003:AACB03000001:abc-200:f"); + } + + @Test + public void rejectsBadStrandLetter() { + assertRejected("A0003:AACB03000001:100-200:x"); + } + + @Test + public void rejectsNull() { + assertRejected(null); + } + + private static void assertRejected(String sourceId) { + try { + StrainSegmentId.parse(sourceId); + fail("expected IllegalArgumentException for: " + sourceId); + } + catch (IllegalArgumentException expected) { + // expected + } + } +} +``` + +- [ ] **Step 2: Run the test to verify it fails** + +```bash +ssh cedar 'bash -lc "cd /var/www/jbrestel.fungidb.org/project_home/ApiCommonWebsite/Model && \ + mvn -q -Dtest=StrainSegmentIdTest -DfailIfNoTests=false test"' +``` + +Expected: **compilation failure** — `cannot find symbol: class StrainSegmentId`. + +If instead you get "No tests were executed", the `src/test/java` tree is not being picked up; confirm the file path matches the package exactly. + +- [ ] **Step 3: Write the implementation** + +Create `StrainSegmentId.java`: + +```java +package org.apidb.apicommon.model.report.bed.util; + +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +/** + * The single parse/format authority for the strain genomic segment primary key. + * + * Grammar: {@code ::-:} + * Example: {@code A0003:AACB03000001:100-200:f} + * + * Coordinates are 1-based, inclusive, and expressed in REFERENCE coordinates. + * Strain coordinates are derived by the attribute query, not stored here. + * + * Note the field patterns are {@code [^:]+} rather than DynSpan's greedy {@code (.*)}. + * DynSpan's SQL and Java parsers disagree on IDs containing extra colons; this one + * rejects them instead of guessing. + */ +public class StrainSegmentId { + + private static final Pattern PATTERN = + Pattern.compile("^([^:]+):([^:]+):(\\d+)-(\\d+):(f|r)$"); + + private final String _strain; + private final String _refSeq; + private final int _refStart; + private final int _refEnd; + private final StrandDirection _strand; + + private StrainSegmentId(String strain, String refSeq, int refStart, int refEnd, + StrandDirection strand) { + _strain = strain; + _refSeq = refSeq; + _refStart = refStart; + _refEnd = refEnd; + _strand = strand; + } + + public static StrainSegmentId parse(String sourceId) { + if (sourceId == null) { + throw new IllegalArgumentException("Strain segment ID may not be null"); + } + Matcher m = PATTERN.matcher(sourceId); + if (!m.matches()) { + throw new IllegalArgumentException(String.format( + "Strain segment ID '%s' does not match required pattern %s", + sourceId, PATTERN.pattern())); + } + int start = Integer.parseInt(m.group(3)); + int end = Integer.parseInt(m.group(4)); + if (start > end) { + throw new IllegalArgumentException(String.format( + "Strain segment ID '%s' has start %d greater than end %d", sourceId, start, end)); + } + return new StrainSegmentId(m.group(1), m.group(2), start, end, + StrandDirection.fromEfOrEr(m.group(5))); + } + + public String format() { + return String.format("%s:%s:%d-%d:%s", _strain, _refSeq, _refStart, _refEnd, + StrandDirection.reverse.equals(_strand) ? "r" : "f"); + } + + /** + * The key into the strain consensus FASTA, and therefore the BED chrom column. + * Written by dnaseq-nextflow as {@code _}. + */ + public String getStrainSeqId() { + return _strain + "_" + _refSeq; + } + + public String getStrain() { return _strain; } + public String getRefSeq() { return _refSeq; } + public int getRefStart() { return _refStart; } + public int getRefEnd() { return _refEnd; } + public StrandDirection getStrand() { return _strand; } +} +``` + +- [ ] **Step 4: Run the test to verify it passes** + +```bash +ssh cedar 'bash -lc "cd /var/www/jbrestel.fungidb.org/project_home/ApiCommonWebsite/Model && \ + mvn -q -Dtest=StrainSegmentIdTest -DfailIfNoTests=false test"' +``` + +Expected: `Tests run: 12, Failures: 0, Errors: 0, Skipped: 0`. + +- [ ] **Step 5: Commit** + +```bash +cd ~/workspaces/fungidb/ApiCommonWebsite +git add Model/src/main/java/org/apidb/apicommon/model/report/bed/util/StrainSegmentId.java \ + Model/src/test/java/org/apidb/apicommon/model/report/bed/util/StrainSegmentIdTest.java +git commit -m "Add StrainSegmentId, the parse/format authority for strain segment PKs" +bash ~/workspaces/agentic-veupath-dev/bin/veup-git-sync.sh fungidb +``` + +--- + +## Task 2: Strain vocabulary param + +The strain list must come from `apidb.indel` (the requirement) and be scoped to the selected organism. There is no strain column on `apidb.indel`; strain is `study.protocolappnode.name` minus a `_Indel` suffix. All 6,119 names carry that suffix, none contains a colon, and none contains any other underscore — so both the strip and the later `_` concatenation are unambiguous. + +The organism param's internal value is `org_abbrev`, which is also the partition key of every `webready.*_p` table. `sharedParams.ChromosomeOrderNumSeq` (`questions/params/sharedParams.xml:2697`) is the working template for an organism-dependent vocabulary query. + +**Files:** +- Create: `ApiCommonModel/Model/lib/wdk/model/questions/params/strainSegmentParams.xml` + +- [ ] **Step 1: Create the param file** + +```xml + + + + + + + Strain or isolate whose coordinate system the returned segment is expressed in. + The list is restricted to strains that have indel data loaded for the selected + organism. + + + + + + + + + + + + + + + + + + + + + + + + + +``` + +- [ ] **Step 2: Verify the vocabulary SQL directly** + +The file is not imported yet, so `wdkQuery` cannot see it. Run the inner SQL by hand for the QA organism (*Aspergillus fumigatus* Af293, `org_abbrev` = `afumAf293`): + +```bash +psql -h localhost -p 5439 -d genomicsdb_rebuild01 -c " +SELECT count(*) AS strains FROM ( + SELECT DISTINCT regexp_replace(pan.name, '_Indel\$', '') AS strain + FROM apidb.indel i, study.protocolappnode pan, webready.GenomicSeqAttributes_p gsa + WHERE pan.protocol_app_node_id = i.protocol_app_node_id + AND gsa.na_sequence_id = i.na_sequence_id + AND gsa.org_abbrev IN ('afumAf293') +) t;" +``` + +Expected: `1117`. If you get `0`, confirm the `org_abbrev` spelling with +`SELECT DISTINCT org_abbrev FROM webready.organismabbreviation_p WHERE organism LIKE 'Aspergillus fumigatus%';` + +Note the cost: this touches roughly 5.1M indel rows for Af293. It is declared +`isCacheable="true"` so it runs once per organism. Record the wall-clock time. If it +exceeds ~30 seconds, note it for follow-up — the fix is a small tuning table keyed on +`(org_abbrev, strain)`, but do not build that speculatively. + +- [ ] **Step 3: Commit** + +```bash +cd ~/workspaces/fungidb/ApiCommonModel +git add Model/lib/wdk/model/questions/params/strainSegmentParams.xml +git commit -m "Add organism-dependent strain vocabulary param for strain segments" +bash ~/workspaces/agentic-veupath-dev/bin/veup-git-sync.sh fungidb +``` + +--- + +## Task 3: ID query with all four validation gates + +Input is a **single** reference location, not a set. Every gate is a filter, so invalid +input yields zero records rather than a broken download — the failure mode +`DynSpansBySegIds` gets wrong by doing no validation at all. + +`sequence_strand` has internal values `f` and `r` (`spanParams.xml:347-363`), so it drops +straight into the PK with no `CASE`. `end_point_segment` documents `0 = end`, which the +`CASE` below honours. + +`project_id` is declared and selected on both branches, mirroring `DynSpansBySourceId` +(`spanQueries.xml:45-46`), which works on UniDB today. The record class excludes +`project_id` from the PK on UniDB; the extra column is tolerated. + +**Files:** +- Create: `ApiCommonModel/Model/lib/wdk/model/questions/queries/strainSegmentQueries.xml` + +- [ ] **Step 1: Create the ID query file** + +```xml + + + + + + + + + + + + + + + + + + = 1 + AND seg.ref_end >= seg.ref_start + AND seg.ref_end <= seg.seq_length + AND EXISTS ( + SELECT 1 + FROM apidb.indel i + , study.protocolappnode pan + WHERE i.protocol_app_node_id = pan.protocol_app_node_id + AND i.na_sequence_id = seg.na_sequence_id + AND regexp_replace(pan.name, '_Indel$', '') = $$strain$$ + ) + ]]> + + + + + + +``` + +- [ ] **Step 2: Verify each gate by hand** + +Still not imported, so run the assembled SQL directly. Substitute a real strain and +contig for Af293 first: + +```bash +psql -h localhost -p 5439 -d genomicsdb_rebuild01 -c " +SELECT gsa.source_id, gsa.na_sequence_id, gsa.length, + regexp_replace(pan.name,'_Indel\$','') AS strain +FROM webready.GenomicSeqAttributes_p gsa + , apidb.indel i, study.protocolappnode pan +WHERE gsa.org_abbrev = 'afumAf293' + AND i.na_sequence_id = gsa.na_sequence_id + AND pan.protocol_app_node_id = i.protocol_app_node_id + AND i.location BETWEEN 100 AND 200 +LIMIT 1;" +``` + +**Record all four values now — Tasks 4 and 7 substitute them.** Write them down: + +| placeholder used later | value from this query | +|---|---| +| `` | `source_id` | +| `` | `na_sequence_id` | +| `` | `strain` | +| sequence length | `length` | + +The `location BETWEEN 100 AND 200` clause is there on purpose: it picks a contig/strain +pair that actually has indels in the 100-200 window, so the Task 7 shift check has a +non-zero offset to prove and does not silently pass on a segment with no indels at all. + +Then run the query body four more times, each with one gate deliberately broken, and +confirm **zero rows** every time: + +| Break | Expect | +|---|---| +| `source_id` set to `'NO_SUCH_CONTIG'` | 0 rows | +| `strain` set to `'NO_SUCH_STRAIN'` | 0 rows | +| `ref_end` set beyond `seq_length` | 0 rows | +| `ref_start` set to `0` | 0 rows | + +A gate that still returns a row is a bug — fix it before moving on. This is the whole +point of the task. + +- [ ] **Step 3: Commit** + +```bash +cd ~/workspaces/fungidb/ApiCommonModel +git add Model/lib/wdk/model/questions/queries/strainSegmentQueries.xml +git commit -m "Add validating ID query for strain genomic segments" +bash ~/workspaces/agentic-veupath-dev/bin/veup-git-sync.sh fungidb +``` + +--- + +## Task 4: Attribute query — reference to strain coordinates + +`shift` is a signed per-event delta (never zero, range −101..+80), so the strain offset at +a reference position is a prefix sum partitioned by `(protocol_app_node_id, na_sequence_id)`. + +Two things make this query's shape non-obvious: + +1. **It cannot use `webready.*_p`.** Attribute queries receive only PK columns, so there + is no `org_abbrev` to prune partitions with, and every index on those tables is + `org_abbrev`-prefixed. There is also no unpartitioned `GenomicSeqAttributes` in this + database — `apidbtuning` exists but does not contain it, which is why DynSpan's `Bfmv` + query cannot run here at all. Instead use the unpartitioned GUS core path: + `dots.externalnasequence` (view; `source_id_uniq` index on `source_id`; carries + `na_sequence_id`, `taxon_id`, `length`) joined to `apidb.organism` and `sres.taxonname`. +2. **`GROUP BY` the resolved node, not just the segment.** If a strain name ever resolves + to two nodes on one sequence, this produces two rows per PK and WDK errors. Joining on + name alone would instead sum two strains' shifts into one plausible-looking wrong + answer with nothing to detect it. + +**Boundary rule:** an indel exactly at `ref_start` lies inside the segment, so it shifts +the end but not the start — `<` for start, `<=` for end. This is the deferred QA item; +implement as written and verify in Task 7. + +**Files:** +- Create: `ApiCommonModel/Model/lib/wdk/model/records/strainSegmentAttributeQueries.xml` + +- [ ] **Step 1: Create the attribute query file** + +```xml + + + + + + + + + + + + + + + + + + + + + + + + + + +``` + +Note `i.protocol_app_node_id` in the `GROUP BY` but not the `SELECT`: that is deliberate. +It is what turns a future name-to-two-nodes collision into duplicate rows and a WDK error +instead of a silently summed wrong offset. + +- [ ] **Step 2: Verify the prefix sum against a hand calculation** + +Pick a strain and contig with indels, then compare. Substitute the values you found in +Task 3 Step 2: + +```bash +psql -h localhost -p 5439 -d genomicsdb_rebuild01 -c " +WITH seg AS (SELECT ''::text AS strain, ::numeric AS na_seq, 100 AS s, 200 AS e) +SELECT SUM(CASE WHEN i.location < seg.s THEN i.shift ELSE 0 END) AS offset_start + , SUM(CASE WHEN i.location <= seg.e THEN i.shift ELSE 0 END) AS offset_end +FROM apidb.indel i, study.protocolappnode pan, seg +WHERE pan.protocol_app_node_id = i.protocol_app_node_id + AND i.na_sequence_id = seg.na_seq + AND regexp_replace(pan.name,'_Indel\$','') = seg.strain + AND i.location <= seg.e;" +``` + +Then verify independently that the offsets are just running sums: + +```bash +psql -h localhost -p 5439 -d genomicsdb_rebuild01 -c " +SELECT i.location, i.shift +FROM apidb.indel i, study.protocolappnode pan +WHERE pan.protocol_app_node_id = i.protocol_app_node_id + AND i.na_sequence_id = + AND regexp_replace(pan.name,'_Indel\$','') = '' + AND i.location <= 200 +ORDER BY i.location;" +``` + +Add the `shift` values by hand: those with `location < 100` must equal `offset_start`, +those with `location <= 200` must equal `offset_end`. They must match exactly. + +- [ ] **Step 3: Commit** + +```bash +cd ~/workspaces/fungidb/ApiCommonModel +git add Model/lib/wdk/model/records/strainSegmentAttributeQueries.xml +git commit -m "Add strain coordinate conversion attribute query" +bash ~/workspaces/agentic-veupath-dev/bin/veup-git-sync.sh fungidb +``` + +--- + +## Task 5: Record class, question, and imports — first build + +This is the first task that can break the build, because it is the task that makes the +model load the new files. + +The record class is deliberately bare: no tables, no summary view, no display attributes, +and only the `bed` reporter. A record-page request should fail rather than render an +empty page. `doNotTest="true"` keeps it out of the WDK sanity tests, matching DynSpan. + +**No `individuals.txt` entry.** Omitting the row is what keeps the search out of the +category tree while leaving it addressable by name through the service API — the same +mechanism `DynSpansBySegIds` and `DynSpansByLocation` already rely on (spec §6.1). + +**Files:** +- Create: `ApiCommonModel/Model/lib/wdk/model/records/strainSegmentRecord.xml` +- Create: `ApiCommonModel/Model/lib/wdk/model/questions/strainSegmentQuestions.xml` +- Modify: `ApiCommonModel/Model/lib/wdk/apiCommonModel.xml` (near lines 419-423) + +- [ ] **Step 1: Create the record class** + +```xml + + + + + + + + + source_id + project_id + + + + + + + + + + + + + + + + + + + + + + + + + +``` + +- [ ] **Step 2: Create the question** + +```xml + + + + + + + + + + + Given a genomic location in reference coordinates and a strain, return the + equivalent segment in that strain's consensus sequence coordinates. + + + + + + + +``` + +- [ ] **Step 3: Register all five files** + +In `Model/lib/wdk/apiCommonModel.xml`, immediately after the existing span imports +(currently lines 419-423), add: + +```xml + + + + + +``` + +Order matters only in that WDK resolves references after parsing all files, so any order +within this block is fine — but keep it grouped and adjacent to the span imports so the +next person finds it. + +- [ ] **Step 4: Build the model** + +```bash +bash ~/workspaces/agentic-veupath-dev/bin/veup-build.sh fungidb wb model +``` + +Expected: build completes and the webapp reloads. + +If it fails, read the error for the specific unresolved reference — a mistyped +`queryRef`, `paramRef`, or column name is the usual cause. A column declared in +`` but absent from the attribute query's `` list will fail here. + +- [ ] **Step 5: Confirm the SQL assembles as WDK will run it** + +```bash +ssh cedar 'bash -lc "source /var/www/jbrestel.fungidb.org/etc/setenv && \ + wdkQuery -model FungiDB -query StrainSegmentAttributes.Coords -showQuery"' +ssh cedar 'bash -lc "source /var/www/jbrestel.fungidb.org/etc/setenv && \ + wdkQuery -model FungiDB -query StrainSegmentId.StrainSegmentsByRefSegment -showParams"' +``` + +Expected: the attribute query prints with `##WDK_ID_SQL##` expanded; the ID query lists +six params (`organismSinglePick`, `strain`, `sequenceId`, `start_point`, +`end_point_segment`, `sequence_strand`). + +- [ ] **Step 6: Commit** + +```bash +cd ~/workspaces/fungidb/ApiCommonModel +git add Model/lib/wdk/model/records/strainSegmentRecord.xml \ + Model/lib/wdk/model/questions/strainSegmentQuestions.xml \ + Model/lib/wdk/apiCommonModel.xml +git commit -m "Add strain segment record class and question, and import the model files" +bash ~/workspaces/agentic-veupath-dev/bin/veup-git-sync.sh fungidb +``` + +--- + +## Task 6: BED feature provider and reporter + +`BedReporter` streams the attributes a provider declares and calls +`getRecordAsBedFields` per record. `GenomicSequenceFeatureProvider` is the precedent for +a provider that computes coordinates from attributes rather than from the PK. + +Two BED columns, two very different constraints: + +- **`chrom` is hard.** `BedLine.bed6`'s first argument *is* the chrom column, and it must + equal `_` or the FASTA index lookup fails. `bed6` also converts start + to 0-based for you — pass 1-based coordinates. +- **`name` is free.** Under `deflineFormat=QUERYONLY`, seqret emits `'>' + name` + verbatim, so this becomes the eventual FASTA defline. `RequestedDeflineFields` is + populated only when the caller passes `deflineType=full`, so the default stays bare. + +**Files:** +- Create: `ApiCommonWebsite/Model/src/main/java/org/apidb/apicommon/model/report/bed/feature/StrainSegmentFeatureProvider.java` +- Create: `ApiCommonWebsite/Model/src/main/java/org/apidb/apicommon/model/report/bed/BedStrainSegmentReporter.java` + +- [ ] **Step 1: Create the feature provider** + +```java +package org.apidb.apicommon.model.report.bed.feature; + +import java.util.List; + +import org.apidb.apicommon.model.report.bed.util.BedLine; +import org.apidb.apicommon.model.report.bed.util.DeflineBuilder; +import org.apidb.apicommon.model.report.bed.util.RequestedDeflineFields; +import org.apidb.apicommon.model.report.bed.util.StrainSegmentId; +import org.gusdb.wdk.model.WdkModelException; +import org.gusdb.wdk.model.record.RecordInstance; +import org.json.JSONObject; + +/** + * Emits one BED line per strain genomic segment. + * + * The chrom column must be the strain consensus FASTA key (_); the name + * column becomes the FASTA defline downstream and carries both coordinate systems. + */ +public class StrainSegmentFeatureProvider implements BedFeatureProvider { + + private static final String ATTR_STRAIN_SEQ_ID = "strain_seq_id"; + private static final String ATTR_STRAIN_START = "strain_start"; + private static final String ATTR_STRAIN_END = "strain_end"; + private static final String ATTR_ORGANISM = "organism"; + + private final RequestedDeflineFields _requestedDeflineFields; + + public StrainSegmentFeatureProvider(JSONObject config) { + _requestedDeflineFields = new RequestedDeflineFields(config); + } + + @Override + public String getRequiredRecordClassFullName() { + return "StrainSegmentRecordClasses.StrainSegmentRecordClass"; + } + + @Override + public String[] getRequiredAttributeNames() { + return new String[] { + ATTR_STRAIN_SEQ_ID, + ATTR_STRAIN_START, + ATTR_STRAIN_END, + ATTR_ORGANISM + }; + } + + @Override + public String[] getRequiredTableNames() { + return new String[0]; + } + + @Override + public List> getRecordAsBedFields(RecordInstance record) throws WdkModelException { + String featureId = getSourceId(record); + + StrainSegmentId id; + try { + id = StrainSegmentId.parse(featureId); + } + catch (IllegalArgumentException e) { + throw new WdkModelException(e.getMessage(), e); + } + + // chrom must be the FASTA key; take it from the attribute rather than recomputing, + // so the model stays the single source of truth for what was looked up. + String strainSeqId = stringValue(record, ATTR_STRAIN_SEQ_ID); + Integer strainStart = integerValueWithZeroForEmpty(record, ATTR_STRAIN_START); + Integer strainEnd = integerValueWithZeroForEmpty(record, ATTR_STRAIN_END); + + if (strainStart < 1 || strainEnd < strainStart) { + throw new WdkModelException(String.format( + "Strain segment %s produced invalid strain coordinates %d-%d", + featureId, strainStart, strainEnd)); + } + + DeflineBuilder defline = new DeflineBuilder(featureId); + + if (_requestedDeflineFields.contains("organism")) { + defline.appendRecordAttribute(record, ATTR_ORGANISM); + } + if (_requestedDeflineFields.contains("strain")) { + defline.appendValue(id.getStrain()); + } + if (_requestedDeflineFields.contains("description")) { + defline.appendValue("segment of strain genomic sequence"); + } + if (_requestedDeflineFields.contains("reference_position")) { + defline.appendPosition(id.getRefSeq(), id.getRefStart(), id.getRefEnd(), id.getStrand()); + } + if (_requestedDeflineFields.contains("position")) { + defline.appendPosition(strainSeqId, strainStart, strainEnd, id.getStrand()); + } + if (_requestedDeflineFields.contains("segment_length")) { + defline.appendSegmentLength(strainStart, strainEnd); + } + + return List.of(BedLine.bed6(strainSeqId, strainStart, strainEnd, defline, id.getStrand())); + } +} +``` + +- [ ] **Step 2: Create the reporter** + +```java +package org.apidb.apicommon.model.report.bed; + +import org.apidb.apicommon.model.report.bed.feature.StrainSegmentFeatureProvider; +import org.gusdb.wdk.model.WdkModelException; +import org.gusdb.wdk.model.report.Reporter; +import org.gusdb.wdk.model.report.ReporterConfigException; +import org.json.JSONObject; + +public class BedStrainSegmentReporter extends BedReporter { + + @Override + public Reporter configure(JSONObject config) throws ReporterConfigException, WdkModelException { + return configure(() -> new StrainSegmentFeatureProvider(config), getContentDisposition(config)); + } + +} +``` + +- [ ] **Step 3: Verify it compiles and the grammar tests still pass** + +```bash +ssh cedar 'bash -lc "cd /var/www/jbrestel.fungidb.org/project_home/ApiCommonWebsite/Model && \ + mvn -q -Dtest=StrainSegmentIdTest -DfailIfNoTests=false test"' +``` + +Expected: `Tests run: 12, Failures: 0` and no compilation errors. + +`BedReporter.configure` validates at runtime that the record class matches +`getRequiredRecordClassFullName()` and that every declared attribute exists on it, so a +typo in an attribute name surfaces on first use, not at compile time. That is what Task 7 +Step 2 checks. + +- [ ] **Step 4: Commit** + +```bash +cd ~/workspaces/fungidb/ApiCommonWebsite +git add Model/src/main/java/org/apidb/apicommon/model/report/bed/feature/StrainSegmentFeatureProvider.java \ + Model/src/main/java/org/apidb/apicommon/model/report/bed/BedStrainSegmentReporter.java +git commit -m "Add BED reporter and feature provider for strain genomic segments" +bash ~/workspaces/agentic-veupath-dev/bin/veup-git-sync.sh fungidb +``` + +--- + +## Task 7: End-to-end verification + +- [ ] **Step 1: Rebuild and confirm registration** + +```bash +bash ~/workspaces/agentic-veupath-dev/bin/veup-build.sh fungidb wb model +``` + +Then, from an already-loaded page on `https://jbrestel.fungidb.org` (a raw curl +307-redirects to autologin), run in the browser console: + +```javascript +await (await fetch('/a/service/record-types/strain-genomic-segment')).json() +``` + +Expected: the record type resolves, `searches` contains `StrainSegmentsByRefSegment`, and +`attributes` lists the nine columns from Task 5. + +- [ ] **Step 2: Confirm it is NOT in the category tree** + +```javascript +const t = await (await fetch('/a/service/ontologies/Categories')).json(); +JSON.stringify(t).includes('StrainSegmentsByRefSegment') +``` + +Expected: **`false`**. If `true`, something added an ontology node — remove it. + +- [ ] **Step 3: Run the search and download BED** + +```javascript +const r = await fetch('/a/service/answer', { + method: 'POST', + headers: {'Content-Type': 'application/json'}, + body: JSON.stringify({ + searchName: 'StrainSegmentsByRefSegment', + searchConfig: {parameters: { + organismSinglePick: 'afumAf293', + strain: '', // from Task 3 Step 2 + sequenceId: '', // from Task 3 Step 2 + start_point: '100', + end_point_segment: '200', + sequence_strand: 'f' + }}, + reportName: 'bed', + reportConfig: {attachmentType: 'plain'} + }) +}); +console.log(await r.text()); +``` + +Expected: one tab-delimited BED line where + +- column 1 (`chrom`) is `_` — **must** match the FASTA defline exactly; +- column 2 is `strain_start - 1` (bed6 converts to 0-based); +- column 3 is `strain_end`; +- column 4 is the source_id (bare, since `deflineType` defaults to short); +- column 6 is `+`. + +If you get `### The result is empty ###`, one of the four gates rejected the input — +re-run the Task 3 Step 2 query to find which. + +- [ ] **Step 4: Prove the coordinates actually shifted** + +Compare the BED output against the reference coordinates you requested: + +```bash +psql -h localhost -p 5439 -d genomicsdb_rebuild01 -c " +SELECT SUM(CASE WHEN i.location < 100 THEN i.shift ELSE 0 END) AS offset_start + , SUM(CASE WHEN i.location <= 200 THEN i.shift ELSE 0 END) AS offset_end +FROM apidb.indel i, study.protocolappnode pan +WHERE pan.protocol_app_node_id = i.protocol_app_node_id + AND i.na_sequence_id = + AND regexp_replace(pan.name,'_Indel\$','') = '' + AND i.location <= 200;" +``` + +BED column 3 must equal `200 + offset_end`, and column 2 must equal +`100 + offset_start - 1`. If `offset_start` and `offset_end` are both non-zero and the +BED coordinates equal the unshifted reference values, the `LEFT JOIN` in Task 4 is not +matching — that is the bug to chase. + +Pick a second case where `offset_start` is `0` but `offset_end` is not (a segment whose +only indels fall inside it) to confirm the two offsets are computed independently. + +- [ ] **Step 5: Check the request logged clean** + +```bash +bash ~/workspaces/agentic-veupath-dev/bin/veup-logs.sh fungidb mark bed1 +# re-run the Step 3 fetch +bash ~/workspaces/agentic-veupath-dev/bin/veup-logs.sh fungidb since bed1 --quiet +``` + +Expected: error logs report `silent:`. Anything in `wdk` or the error logs is a real +problem even if the BED output looked right. + +- [ ] **Step 6: Verify the defline fields work** + +Re-run Step 3 with `reportConfig` set to: + +```javascript +{attachmentType: 'plain', deflineType: 'full', + deflineFields: ['organism','strain','reference_position','position','segment_length']} +``` + +Expected: BED column 4 becomes a pipe-delimited defline carrying organism, strain, both +coordinate systems, and the length — confirming both `ref_*` and `strain_*` values are +present and different. + +- [ ] **Step 7: Commit any fixes** + +```bash +cd ~/workspaces/fungidb/ApiCommonModel # or ApiCommonWebsite +git add -u && git commit -m "Fix found in end-to-end verification" +bash ~/workspaces/agentic-veupath-dev/bin/veup-git-sync.sh fungidb +``` + +--- + +## Task 8: Include UniDB on the existing eQTL genomic-segment search + +Independent of everything above, and safe to do first or last. The spec requires UniDB be +included for **existing** genomic segments as well as the new record. + +Background you need: the portal site deploys as tomcat project `EuPathDB` but its **WDK +model name is `UniDB`**, and `includeProjects` matches the *model* name. So +`includeProjects="EuPathDB"` never fires on that site. Repo-wide there are 42 files still +carrying legacy `EuPathDB` in `includeProjects` versus 760 `UniDB` occurrences — **do not +try to fix them all**, that is a separate cleanup. Only the genomic-segment files are in +scope, and in those there is exactly one live defect. + +Audit of `EuPathDB` in the span model files, so you can see why only one line changes: + +| Location | Verdict | +|---|---| +| `spanQuestions.xml:346` | inside a commented-out `IsolatesBySpanLogic` block — dead, leave it | +| `spanQuestions.xml:478` | `includeProjects="PlasmoDB,EuPathDB,UniDB"` — already has UniDB, harmless | +| `spanQuestions.xml:489` | **live defect** — `attributesList` scoped to `EuPathDB` only, so UniDB gets no summary list | +| `spanQuestions.xml:634` | an `excludeProjects` that does not name UniDB, so UniDB stays included — which is what we want | + +**Files:** +- Modify: `ApiCommonModel/Model/lib/wdk/model/questions/spanQuestions.xml:489` + +- [ ] **Step 1: Confirm the defect before changing it** + +```bash +sed -n '478,495p' ~/workspaces/fungidb/ApiCommonModel/Model/lib/wdk/model/questions/spanQuestions.xml +``` + +Expected: question `DynSpansByEQTLtoGenes` includes `UniDB`, but the second +`` is scoped `includeProjects="EuPathDB"`. Confirm the two +`` bodies differ only in `sorting` whitespace — if they differ +substantively, stop and ask, because then the intent was not a simple rename. + +- [ ] **Step 2: Add UniDB to the attributesList scope** + +Change line 489 from: + +```xml + Date: Fri, 31 Jul 2026 11:57:55 -0400 Subject: [PATCH 04/42] Drop all project scoping from the new strain segment record The new record works on every project, so it carries no includeProjects or excludeProjects anywhere: not on the record class, the question, the queries, or the primary key columns. This removes an excludeProjects="UniDB" on the project_id PK column and the associated UniDB query variants, both carried over from DynSpan without justification. DynSpan needs that fork because it derives project_id from @PROJECT_ID@, which is meaningless on the portal. This record takes project_id from webready.GenomicSeqAttributes_p instead, so one query and one PK are correct everywhere. Unaffected: Task 8, which adds UniDB to the pre-existing eQTL segment search's attributesList scope. That concerns an existing search, not this record. Co-Authored-By: Claude Opus 5 --- .../plans/2026-07-31-strain-segment-record.md | 15 +++++++++----- ...2026-07-31-strain-segment-record-design.md | 20 +++++++++++++++---- 2 files changed, 26 insertions(+), 9 deletions(-) diff --git a/docs/superpowers/plans/2026-07-31-strain-segment-record.md b/docs/superpowers/plans/2026-07-31-strain-segment-record.md index 6c3f4af4f..142c42c03 100644 --- a/docs/superpowers/plans/2026-07-31-strain-segment-record.md +++ b/docs/superpowers/plans/2026-07-31-strain-segment-record.md @@ -410,9 +410,12 @@ input yields zero records rather than a broken download — the failure mode straight into the PK with no `CASE`. `end_point_segment` documents `0 = end`, which the `CASE` below honours. -`project_id` is declared and selected on both branches, mirroring `DynSpansBySourceId` -(`spanQueries.xml:45-46`), which works on UniDB today. The record class excludes -`project_id` from the PK on UniDB; the extra column is tolerated. +**One `` block, no project variants.** Do not add `includeProjects` or +`excludeProjects` to anything in this task. `project_id` is selected from +`webready.GenomicSeqAttributes_p`, not from `@PROJECT_ID@`, so it is correct on every +project including UniDB. DynSpan duplicates its queries for UniDB precisely because it +uses `@PROJECT_ID@`, which is meaningless on the portal — that is a problem this design +avoids rather than a pattern to copy. **Files:** - Create: `ApiCommonModel/Model/lib/wdk/model/questions/queries/strainSegmentQueries.xml` @@ -730,7 +733,9 @@ mechanism `DynSpansBySegIds` and `DynSpansByLocation` already rely on (spec §6. + Not user facing: no ontology node, no record page, no saved strategies. + Works on every project: no includeProjects/excludeProjects anywhere, including + on the PK columns. project_id comes from the data, not @PROJECT_ID@. --> source_id - project_id + project_id diff --git a/docs/superpowers/specs/2026-07-31-strain-segment-record-design.md b/docs/superpowers/specs/2026-07-31-strain-segment-record-design.md index e1b323710..46bd0177b 100644 --- a/docs/superpowers/specs/2026-07-31-strain-segment-record-design.md +++ b/docs/superpowers/specs/2026-07-31-strain-segment-record-design.md @@ -109,10 +109,22 @@ source_id = ::-: A0003:AACB03000001:100-200:f ``` -Primary key columns: `source_id`, plus `project_id` with `excludeProjects="UniDB"` and -UniDB-specific `` variants, mirroring the existing fork at -`dynSpanAttributeQueries.xml:16-33`. **UniDB is included** for this record and is to be -added to the existing genomic-segment `includeProjects` lists where absent. +Primary key columns: `source_id` and `project_id`, uniformly. + +**No `includeProjects` or `excludeProjects` anywhere** — not on the record class, the +question, the queries, or the PK columns. The record works on every project, UniDB +included. + +This is a deliberate departure from DynSpan, which carries an `excludeProjects="UniDB"` PK +column *and* duplicated UniDB `` variants (`dynSpanAttributeQueries.xml:16-33`). That +fork exists because DynSpan derives `project_id` from `@PROJECT_ID@`, which is meaningless +on the portal. This record instead takes `project_id` from the data +(`webready.GenomicSeqAttributes_p.project_id`), so it is populated correctly on every +project including UniDB and needs no fork. One query, one PK, no variants. + +Note this is separate from the requirement to add UniDB to the **existing** +genomic-segment `includeProjects` lists where absent — that concerns pre-existing +searches, not this record. Coordinates in the PK are **reference** coordinates — the input, not the output. Strain coordinates are derived in the attribute query (§5.2). Rationale: From 84f97359d714335f895970dd03413e80217bb84f Mon Sep 17 00:00:00 2001 From: John Brestelli Date: Fri, 31 Jul 2026 12:28:48 -0400 Subject: [PATCH 05/42] Sync spec and plan with the shipped StrainSegmentId grammar Code review of Task 1 tightened the implementation in three ways that the design documents did not yet reflect. The strain capture group now excludes underscore. The FASTA key is _ and reference sequence IDs legitimately contain underscores (Pf3D7_01_v3), so without that exclusion strain A_B with sequence C and strain A with sequence B_C both yield the key A_B_C. No current strain name contains an underscore, so this is inert today and turns a future ambiguous key into a parse failure instead of a silently unfindable sequence. Validation moved into the private constructor and gained a refStart < 1 check. A zero start reaches BedLine.locationToZeroBased() and emits chromStart = -1, a malformed BED line with nothing pointing back at the ID that caused it. The original plan only checked start > end. Co-Authored-By: Claude Opus 5 --- .../plans/2026-07-31-strain-segment-record.md | 9 ++++++++ ...2026-07-31-strain-segment-record-design.md | 22 +++++++++++++++++++ 2 files changed, 31 insertions(+) diff --git a/docs/superpowers/plans/2026-07-31-strain-segment-record.md b/docs/superpowers/plans/2026-07-31-strain-segment-record.md index 142c42c03..333aca613 100644 --- a/docs/superpowers/plans/2026-07-31-strain-segment-record.md +++ b/docs/superpowers/plans/2026-07-31-strain-segment-record.md @@ -72,6 +72,15 @@ One responsibility each. The grammar class is the only piece with no WDK depende ## Task 1: The PK grammar class +> **COMPLETE — and amended after code review.** Commits `19defeba2` (as written below) +> and `3ead3adac` (review fixes). The code block in Step 3 is what was *planned*; the +> shipped version differs in three ways, all worth knowing if you touch this class: +> the regex is `^([^:_]+):([^:]+):(\d+)-(\d+):(f|r)$` (strain excludes `_`, because +> `_` would otherwise be ambiguous — sequence IDs contain underscores); +> validation lives in the private constructor rather than `parse()`, and adds a +> `refStart < 1` check; and both `parseInt` calls rethrow with the full ID in the +> message. 14 tests, not 12. See spec §3.1 for the reasoning. + The `ApiCommonWebsite/Model` module has JUnit 4 on its classpath but no `src/test/java` tree yet. You are creating it. Standard Maven layout means Surefire picks it up with no POM change. **Files:** diff --git a/docs/superpowers/specs/2026-07-31-strain-segment-record-design.md b/docs/superpowers/specs/2026-07-31-strain-segment-record-design.md index 46bd0177b..4f4553589 100644 --- a/docs/superpowers/specs/2026-07-31-strain-segment-record-design.md +++ b/docs/superpowers/specs/2026-07-31-strain-segment-record-design.md @@ -151,6 +151,28 @@ feature provider. On the SQL side use `split_part(source_id, ':', N)`, not posit contain `-` (`A17-48H-7`), which is safe only because the range is its own colon field; the range must never be parsed out of the whole string positionally. +**As implemented** (commits `19defeba2`, `3ead3adac`): + +``` +^([^:_]+):([^:]+):(\d+)-(\d+):(f|r)$ +``` + +Two details of that pattern are load-bearing: + +- Fields are `[^:]+`, not DynSpan's greedy `(.*)`, so an ID carrying an extra colon is + **rejected** rather than mis-parsed into a different segment. +- The **strain** group additionally excludes `_`, because the FASTA key is + `_` and reference sequence IDs legitimately contain underscores + (`Pf3D7_01_v3`). Without that exclusion, strain `A_B` + sequence `C` and strain `A` + + sequence `B_C` would both yield the key `A_B_C`. No current strain name contains an + underscore (0 of 6,119), so this is inert today and converts a future ambiguous key + into a loud parse failure. Group 2 stays `[^:]+`. + +Validation lives in the private constructor, not in `parse()`, so that any later +from-parts factory cannot bypass it. It rejects `refStart < 1` (a 0 start would reach +`BedLine.locationToZeroBased()` and emit `chromStart = -1`, a malformed BED line), +`refEnd < refStart`, and any strand other than forward or reverse. + ## 4. Where DynSpan gets validation wrong Recorded because the new search must not repeat it. From 37350e58e9750d027597e78e822eecb014cc0bc2 Mon Sep 17 00:00:00 2001 From: John Brestelli Date: Fri, 31 Jul 2026 12:30:53 -0400 Subject: [PATCH 06/42] Add organism-dependent strain vocabulary param for strain segments --- .../questions/params/strainSegmentParams.xml | 54 +++++++++++++++++++ 1 file changed, 54 insertions(+) create mode 100644 Model/lib/wdk/model/questions/params/strainSegmentParams.xml diff --git a/Model/lib/wdk/model/questions/params/strainSegmentParams.xml b/Model/lib/wdk/model/questions/params/strainSegmentParams.xml new file mode 100644 index 000000000..6ef591633 --- /dev/null +++ b/Model/lib/wdk/model/questions/params/strainSegmentParams.xml @@ -0,0 +1,54 @@ + + + + + + + Strain or isolate whose coordinate system the returned segment is expressed in. + The list is restricted to strains that have indel data loaded for the selected + organism. + + + + + + + + + + + + + + + + + + + + + + + + + From fb5666e4e82ef9f752698094ce46b0da2708249c Mon Sep 17 00:00:00 2001 From: John Brestelli Date: Fri, 31 Jul 2026 12:39:59 -0400 Subject: [PATCH 07/42] Dedupe strain vocabulary on node id and drop the project-restricted organism vocab --- .../questions/params/strainSegmentParams.xml | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/Model/lib/wdk/model/questions/params/strainSegmentParams.xml b/Model/lib/wdk/model/questions/params/strainSegmentParams.xml index 6ef591633..bf407c310 100644 --- a/Model/lib/wdk/model/questions/params/strainSegmentParams.xml +++ b/Model/lib/wdk/model/questions/params/strainSegmentParams.xml @@ -27,8 +27,7 @@ organism gate and the partition key of webready.GenomicSeqAttributes_p. --> + quote="true"/> @@ -37,12 +36,16 @@ SELECT strain AS internal, strain AS term, strain AS display FROM ( SELECT DISTINCT regexp_replace(pan.name, '_Indel$', '') AS strain - FROM apidb.indel i + FROM ( + SELECT DISTINCT i.protocol_app_node_id + FROM apidb.indel i + , webready.GenomicSeqAttributes_p gsa + WHERE gsa.na_sequence_id = i.na_sequence_id + AND gsa.org_abbrev IN ($$organismSinglePick$$) + ) x , study.protocolappnode pan - , webready.GenomicSeqAttributes_p gsa - WHERE pan.protocol_app_node_id = i.protocol_app_node_id - AND gsa.na_sequence_id = i.na_sequence_id - AND gsa.org_abbrev IN ($$organismSinglePick$$) + WHERE pan.protocol_app_node_id = x.protocol_app_node_id + AND pan.name LIKE '%\_Indel' ) t ORDER BY strain ]]> From b9d7872fa2d1d33dc3b76f892d8cb79f83887404 Mon Sep 17 00:00:00 2001 From: John Brestelli Date: Fri, 31 Jul 2026 12:46:40 -0400 Subject: [PATCH 08/42] Add validating ID query for strain genomic segments --- .../queries/strainSegmentQueries.xml | 62 +++++++++++++++++++ 1 file changed, 62 insertions(+) create mode 100644 Model/lib/wdk/model/questions/queries/strainSegmentQueries.xml diff --git a/Model/lib/wdk/model/questions/queries/strainSegmentQueries.xml b/Model/lib/wdk/model/questions/queries/strainSegmentQueries.xml new file mode 100644 index 000000000..9936cb321 --- /dev/null +++ b/Model/lib/wdk/model/questions/queries/strainSegmentQueries.xml @@ -0,0 +1,62 @@ + + + + + + + + + + + + + + + + + + = 1 + AND seg.ref_end >= seg.ref_start + AND seg.ref_end <= seg.seq_length + AND EXISTS ( + SELECT 1 + FROM apidb.indel i + , study.protocolappnode pan + WHERE i.protocol_app_node_id = pan.protocol_app_node_id + AND i.na_sequence_id = seg.na_sequence_id + AND pan.name = CONCAT($$strain$$, '_Indel') + ) + ]]> + + + + + + From f91996c26c7f6f011c8b37984006971a63d457a2 Mon Sep 17 00:00:00 2001 From: John Brestelli Date: Fri, 31 Jul 2026 12:53:08 -0400 Subject: [PATCH 09/42] Remove the organism param: a sequence ID already determines its organism MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The search is handed a reference sequence ID, so also being handed an organism was redundant input that only created a consistency question the query then had to answer. Dropping it removes three problems at once. No partition-key concern: the ID query now resolves sequences through unpartitioned dots.ExternalNaSequence rather than webready.GenomicSeqAttributes_p, which is the same table path the attribute query already used, so the two are consistent instead of divergent. This design now touches no webready.*_p table. No @PROJECT_ID@: project_id for the primary key comes from apidb.organism.project_name via taxon_id, so one query stays correct on every project. No vocabulary-shape trap. With an organism param, org_abbrev IN ($$organismSinglePick$$) is wrong, and silently so. That param defaults to organismVQ.withGenes, a tree vocabulary projecting string_agg(abbrev, ', '), so a leaf term yields one abbrev but any grouping node yields 'afumAf293, afumA1163' — a single quoted literal matching no row, with no error logged. The usual dodge is overriding queryRef to a flat abbrev vocabulary, but every such vocabulary carries project scoping, which this record may not have. The original requirement to validate the reference sequence against the reference organism is now satisfied by construction: the EXISTS gate proves the strain has indel data on that exact sequence, so strain and sequence are consistent without trusting a supplied organism. Strain vocabulary becomes global: 6,119 strains in 2.9s, cached once instead of once per organism, so cheaper in aggregate than the 2.66s-per-organism version. Co-Authored-By: Claude Opus 5 --- .../plans/2026-07-31-strain-segment-record.md | 46 +++++++- ...2026-07-31-strain-segment-record-design.md | 102 ++++++++++++++---- 2 files changed, 120 insertions(+), 28 deletions(-) diff --git a/docs/superpowers/plans/2026-07-31-strain-segment-record.md b/docs/superpowers/plans/2026-07-31-strain-segment-record.md index 333aca613..063f4a7fb 100644 --- a/docs/superpowers/plans/2026-07-31-strain-segment-record.md +++ b/docs/superpowers/plans/2026-07-31-strain-segment-record.md @@ -311,7 +311,13 @@ bash ~/workspaces/agentic-veupath-dev/bin/veup-git-sync.sh fungidb The strain list must come from `apidb.indel` (the requirement) and be scoped to the selected organism. There is no strain column on `apidb.indel`; strain is `study.protocolappnode.name` minus a `_Indel` suffix. All 6,119 names carry that suffix, none contains a colon, and none contains any other underscore — so both the strip and the later `_` concatenation are unambiguous. -The organism param's internal value is `org_abbrev`, which is also the partition key of every `webready.*_p` table. `sharedParams.ChromosomeOrderNumSeq` (`questions/params/sharedParams.xml:2697`) is the working template for an organism-dependent vocabulary query. +> **REVISED after review — there is no organism param.** An earlier draft made this +> vocabulary depend on an organism param and scope itself with `org_abbrev`. That was +> redundant: the search is handed a sequence ID, which already determines its organism. +> The vocabulary is now **global** (6,119 strains, 2.9s, cached once rather than once per +> organism), and relevance is enforced by the ID query's `EXISTS` gate instead. See spec +> §5.1.1 for why, including the tree-vocabulary trap that made the organism-scoped version +> silently return nothing. The Step 1 content below is the revised version. **Files:** - Create: `ApiCommonModel/Model/lib/wdk/model/questions/params/strainSegmentParams.xml` @@ -419,6 +425,37 @@ input yields zero records rather than a broken download — the failure mode straight into the PK with no `CASE`. `end_point_segment` documents `0 = end`, which the `CASE` below honours. +**Do not add `queryRef="organismVQ.withStrainsChromosome"` to the organism `paramRef`**, +even though `DynSpansBySourceId` does. That query is declared for only 10 projects, so +referencing it forces an `includeProjects` guard onto everything that depends on it — and +this record is meant to work on all projects. Worse, its SQL selects from +`apidbtuning.GenomicSeqAttributes`, which **does not exist in this database** (verified +2026-07-31; the same absence that stops DynSpan's `Bfmv` query from running here). Let +`organismParams.organismSinglePick` use its own default vocab query, `organismVQ.withGenes`, +which carries no `includeProjects`. + +Note `organismSinglePick` is declared `multiPick="true" maxSelectedCount="1"` +(`organismParams.xml:256-262`), so WDK substitutes a quoted comma-separated list even +though only one value can be chosen. `IN (...)` is therefore required and `=` would be a +bug. + +**Param quoting — write params UNQUOTED in the SQL.** This codebase quotes enum and vocab +params by default; `spanQueries.xml:303` has to say `quote="false"` explicitly to turn it +off. So `$$sequence_strand$$` and `$$strain$$` arrive already wrapped in single quotes, and +adding your own would produce `''f''`. Set `quote="true"` on the `strain` paramRef to make +the intent explicit, then reference both bare. + +The codebase is genuinely inconsistent here (`spanQueries.xml:400` uses bare +`$$liberal_conservative$$` while `:417` writes `'$$any_or_all_DynSeg$$'`), so this cannot be +settled by reading alone. **Task 5 Step 5 is where it gets confirmed**: `wdkQuery -showQuery` +renders the assembled SQL, and doubled quotes will be visible there. If they appear, remove +the `quote="true"` rather than adding literal quotes. + +Also prefer `pan.name = CONCAT($$strain$$, '_Indel')` over `regexp_replace(pan.name, ...) += $$strain$$` in the EXISTS gate. Same lesson as Task 2: comparing a plain string lets the +database use an index on `name`, whereas applying `regexp_replace` to every candidate row +forces a scan and computes a function per row to reach the same answer. + **One `` block, no project variants.** Do not add `includeProjects` or `excludeProjects` to anything in this task. `project_id` is selected from `webready.GenomicSeqAttributes_p`, not from `@PROJECT_ID@`, so it is correct on every @@ -445,10 +482,9 @@ avoids rather than a pattern to copy. names map to more than one organism. --> - + @@ -461,7 +497,7 @@ avoids rather than a pattern to copy. diff --git a/docs/superpowers/specs/2026-07-31-strain-segment-record-design.md b/docs/superpowers/specs/2026-07-31-strain-segment-record-design.md index 4f4553589..ff0a12c9e 100644 --- a/docs/superpowers/specs/2026-07-31-strain-segment-record-design.md +++ b/docs/superpowers/specs/2026-07-31-strain-segment-record-design.md @@ -88,6 +88,12 @@ corruption. > is `org_abbrev`, not `organism_abbrev`. Omitting it scans all partitions (831 on > `organismabbreviation_p`) and uses no index. +**As it turns out, this design touches no `webready.*_p` table at all** — see §5.1.1. Both +the ID query and the attribute query resolve sequences through unpartitioned +`dots.ExternalNaSequence`, so the partition-key rule above never binds. It is recorded +because it drove an earlier draft and because anyone extending this record needs to know it +applies the moment they reach for a `_p` table. + `organism -> org_abbrev` comes from `webready.organismabbreviation_p` (`organism`, `org_abbrev`, `project_id`, `sanitized_org_abbrev`, `name_for_filenames`). @@ -192,39 +198,89 @@ Input is a **single reference location**, modeled on `DynSpansByLocation` (`spanQueries.xml:266`), not a `datasetParam` — so `SpanParams.span_id` and its `recordClassRef` to DynSpan are not involved. -Params: `organism` (single pick), `strain` (enum, **dependent on organism**), -`sequenceId`, `start`, `end`, `strand`. +Params: `strain`, `sequenceId`, `start`, `end`, `strand`. **There is no organism param.** + +### 5.1.1 Why no organism input + +An earlier draft took an organism as a first param, used it to prune the `org_abbrev` +partition key and to check that the reference sequence belonged to it. That was redundant +input: **a reference sequence ID already determines its organism.** Being handed an +organism as well only creates a consistency question the query then has to answer. + +Dropping it removes three problems at once: + +- **No partition-key concern.** Resolve the sequence through unpartitioned + `dots.ExternalNaSequence` (`source_id_uniq` index; carries `na_sequence_id`, `taxon_id`, + `length`) rather than `webready.GenomicSeqAttributes_p`. Same table path §5.3 already + uses, so the two queries become consistent instead of divergent. +- **No `@PROJECT_ID@`.** `project_id` for the PK comes from `apidb.organism.project_name` + via `taxon_id` — unpartitioned, correct on every project, still one query. +- **No vocabulary-shape trap.** See the note below; this is the defect that prompted the + redesign. -The organism param's **internal value is `org_abbrev`**, so the partition key is -available directly to every query without an extra lookup. Precedent: -`organismParams.organism_span` is compared straight against `org_abbrev` columns at -`spanQueries.xml:203-205`. The display organism name, when needed for a defline or a -join, comes from `webready.organismabbreviation_p` keyed on the same `org_abbrev`. +> **The trap, recorded because it cost a round of rework.** With an organism param, the +> obvious `org_abbrev IN ($$organismSinglePick$$)` is wrong. That param defaults to +> `organismVQ.withGenes` (`organismParams.xml:677`), a *tree* vocabulary projecting +> `string_agg(fq.abbrev, ', ') AS internal` grouped by `term, parentTerm` — so a leaf term +> gives one abbrev but any grouping node gives `'afumAf293, afumA1163'`. Quoted, that is a +> single literal matching **nothing**: verified, `org_abbrev IN ('afumAf293, afumA1163')` +> returns 0 rows, with no error and no log line. Most of the model dodges this by overriding +> `queryRef` to a flat abbrev vocabulary, but every such vocabulary carries +> `includeProjects`/`excludeProjects`, which this record may not have. Not taking an +> organism at all sidesteps the whole question. -Three gates, all as joins/filters so bad input yields **zero records** rather than a -broken download: +Three gates, all as filters so bad input yields **zero records** rather than a broken +download: -1. `strain` is in the vocabulary for the chosen organism (§5.2 query); -2. `refSeq` belongs to the chosen organism — join `webready.genomicseqattributes_p` - constraining `org_abbrev` to the chosen organism, which is both the partition key and - the organism-membership check that `DynSpansBySourceId` omits; -3. `1 <= refStart <= refEnd <= gsa.length`. +1. the reference sequence exists — `dots.ExternalNaSequence.source_id = $$sequenceId$$`; +2. `1 <= refStart <= refEnd <= ens.length`; +3. the strain has indel data on **this** sequence — an `EXISTS` over `apidb.indel` joined to + `study.protocolappnode`, matched on `na_sequence_id`. -### 5.2 Strain vocabulary (dependent param) +Gate 3 subsumes the organism check the earlier draft did explicitly: if the strain has indel +rows on that sequence, strain and sequence are consistent **by construction**. That is a +stronger guarantee than validating against a user-supplied organism, and it is what the +original requirement ("validate the reference sequence against the reference organism") +actually wanted. + +Strain membership itself is enforced by WDK, which validates a `flatVocabParam` value +against its vocabulary — that is what makes strain names a controlled vocabulary sourced +from `apidb.indel`, as required. + +### 5.2 Strain vocabulary (global, not organism-dependent) + +With no organism param there is nothing to depend on, so the vocabulary is global: ```sql -SELECT DISTINCT regexp_replace(pan.name, '_Indel$', '') AS strain -FROM apidb.indel i - JOIN study.protocolappnode pan ON pan.protocol_app_node_id = i.protocol_app_node_id - JOIN webready.genomicseqattributes_p gsa ON gsa.na_sequence_id = i.na_sequence_id -WHERE gsa.org_abbrev = $$organism$$ -- partition key; param's internal value +SELECT strain AS internal, strain AS term, strain AS display +FROM ( + SELECT DISTINCT regexp_replace(pan.name, '_Indel$', '') AS strain + FROM (SELECT DISTINCT protocol_app_node_id FROM apidb.indel) x + , study.protocolappnode pan + WHERE pan.protocol_app_node_id = x.protocol_app_node_id + AND pan.name LIKE '%\_Indel' +) t +ORDER BY strain ``` -(`$$organism$$` is the `org_abbrev` internal value per §5.1, so this both scopes the -vocabulary to the chosen organism and hits the partition.) +**6,119 strains in 2.9s**, measured 2026-07-31. Cached once globally rather than once per +organism, so cheaper in aggregate than the per-organism version it replaces (2.66s each). + +Two shape details carried over from review, both load-bearing: + +- Dedupe on the numeric `protocol_app_node_id` (6,249 values) **before** computing + `regexp_replace`, not after. Deduping on the regexp'd string instead runs the function + across all 43.6M indel rows and hash-aggregates text keys — measured 12,085ms versus + 2,681ms for identical output. +- `pan.name LIKE '%\_Indel'` makes the suffix invariant executable rather than merely + documented; `regexp_replace` silently passes a non-conforming name through, which would + become a bogus strain option. The backslash escape matters — `_` is a single-character + wildcard in `LIKE`. This is the controlled vocabulary required by the brief: strain names come from -`apidb.indel`, scoped per organism. No tuning table exists for it. +`apidb.indel` (via `study.protocolappnode`, which is where the name actually lives). No +tuning table exists for it. Scoping to a *relevant* strain is not the vocabulary's job — +gate 3 of §5.1 rejects a strain with no data on the requested sequence. ### 5.3 Attribute query: `StrainSegmentAttributes.Coords` From dce329e3cad11dc8d3556aad37cde325ceca6a56 Mon Sep 17 00:00:00 2001 From: John Brestelli Date: Fri, 31 Jul 2026 12:56:22 -0400 Subject: [PATCH 10/42] Drop the organism param from strain segment queries A reference sequence ID already determines its organism, so taking an organism as well was redundant input. Resolving the sequence through unpartitioned dots.ExternalNaSequence removes the partition-key constraint, and taking project_id from apidb.organism.project_name avoids @PROJECT_ID@. The strain vocabulary becomes global; relevance is enforced by the ID query's EXISTS gate. --- .../questions/params/strainSegmentParams.xml | 22 ++++------ .../queries/strainSegmentQueries.xml | 42 +++++++++++-------- 2 files changed, 31 insertions(+), 33 deletions(-) diff --git a/Model/lib/wdk/model/questions/params/strainSegmentParams.xml b/Model/lib/wdk/model/questions/params/strainSegmentParams.xml index bf407c310..f61371ed0 100644 --- a/Model/lib/wdk/model/questions/params/strainSegmentParams.xml +++ b/Model/lib/wdk/model/questions/params/strainSegmentParams.xml @@ -5,12 +5,10 @@ + multiPick="false"> Strain or isolate whose coordinate system the returned segment is expressed in. - The list is restricted to strains that have indel data loaded for the selected - organism. + The list is all strains that have indel data loaded. @@ -23,11 +21,11 @@ + no strain column on apidb.indel. The vocabulary is global: the search + takes no organism param, and relevance is enforced instead by the ID + query's EXISTS gate, which scopes a chosen strain to the specific reference + sequence it has indel data on. --> - @@ -36,13 +34,7 @@ SELECT strain AS internal, strain AS term, strain AS display FROM ( SELECT DISTINCT regexp_replace(pan.name, '_Indel$', '') AS strain - FROM ( - SELECT DISTINCT i.protocol_app_node_id - FROM apidb.indel i - , webready.GenomicSeqAttributes_p gsa - WHERE gsa.na_sequence_id = i.na_sequence_id - AND gsa.org_abbrev IN ($$organismSinglePick$$) - ) x + FROM (SELECT DISTINCT protocol_app_node_id FROM apidb.indel) x , study.protocolappnode pan WHERE pan.protocol_app_node_id = x.protocol_app_node_id AND pan.name LIKE '%\_Indel' diff --git a/Model/lib/wdk/model/questions/queries/strainSegmentQueries.xml b/Model/lib/wdk/model/questions/queries/strainSegmentQueries.xml index 9936cb321..e23ec959f 100644 --- a/Model/lib/wdk/model/questions/queries/strainSegmentQueries.xml +++ b/Model/lib/wdk/model/questions/queries/strainSegmentQueries.xml @@ -2,17 +2,22 @@ - + - @@ -29,18 +34,19 @@ $$sequence_strand$$) AS source_id , seg.project_id FROM ( - SELECT gsa.source_id AS ref_seq - , gsa.project_id + SELECT ens.source_id AS ref_seq + , o.project_name AS project_id , $$start_point$$::integer AS ref_start , CASE WHEN $$end_point_segment$$::integer = 0 - THEN gsa.length::integer + THEN ens.length::integer ELSE $$end_point_segment$$::integer END AS ref_end - , gsa.length::integer AS seq_length - , gsa.na_sequence_id - FROM webready.GenomicSeqAttributes_p gsa - WHERE gsa.org_abbrev IN ($$organismSinglePick$$) - AND gsa.source_id = $$sequenceId$$ + , ens.length::integer AS seq_length + , ens.na_sequence_id + FROM dots.ExternalNaSequence ens + , apidb.organism o + WHERE o.taxon_id = ens.taxon_id + AND ens.source_id = $$sequenceId$$ ) seg WHERE seg.ref_start >= 1 AND seg.ref_end >= seg.ref_start From 1fd3f08407c2fee4f6fd8ef3a22cc93357c10dac Mon Sep 17 00:00:00 2001 From: John Brestelli Date: Fri, 31 Jul 2026 13:02:19 -0400 Subject: [PATCH 11/42] Add strain coordinate conversion attribute query --- .../records/strainSegmentAttributeQueries.xml | 88 +++++++++++++++++++ 1 file changed, 88 insertions(+) create mode 100644 Model/lib/wdk/model/records/strainSegmentAttributeQueries.xml diff --git a/Model/lib/wdk/model/records/strainSegmentAttributeQueries.xml b/Model/lib/wdk/model/records/strainSegmentAttributeQueries.xml new file mode 100644 index 000000000..9ae0134b8 --- /dev/null +++ b/Model/lib/wdk/model/records/strainSegmentAttributeQueries.xml @@ -0,0 +1,88 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + From 2589a39b0722fe88a10c33715b15c70f6bbb64fd Mon Sep 17 00:00:00 2001 From: John Brestelli Date: Fri, 31 Jul 2026 13:11:09 -0400 Subject: [PATCH 12/42] Drive strain vocabulary from protocolappnode and clarify query comments Deriving from study.protocolappnode with an EXISTS probe into indel_ix1 replaces a scan of all 43.6M apidb.indel rows: 2802ms to 57ms for an identical 6119-strain result set. Renames the query, whose StrainsByOrganism name no longer described it. Records why CONCAT is applied to the parameter rather than the column, and that the apidb.organism join is also a gate. --- .../questions/params/strainSegmentParams.xml | 24 +++++++---- .../queries/strainSegmentQueries.xml | 40 +++++++++++++------ 2 files changed, 45 insertions(+), 19 deletions(-) diff --git a/Model/lib/wdk/model/questions/params/strainSegmentParams.xml b/Model/lib/wdk/model/questions/params/strainSegmentParams.xml index f61371ed0..e231031dc 100644 --- a/Model/lib/wdk/model/questions/params/strainSegmentParams.xml +++ b/Model/lib/wdk/model/questions/params/strainSegmentParams.xml @@ -3,7 +3,7 @@ @@ -24,8 +24,18 @@ no strain column on apidb.indel. The vocabulary is global: the search takes no organism param, and relevance is enforced instead by the ID query's EXISTS gate, which scopes a chosen strain to the specific reference - sequence it has indel data on. --> - + sequence it has indel data on. + + Driven from study.protocolappnode (61,512 rows) with an EXISTS probe into + apidb.indel, rather than the reverse: an earlier revision derived the + candidate set from a DISTINCT scan of all 43.6M apidb.indel rows, which + dominated its cost. Probing indel from the protocolappnode side lets the + planner use indel's protocol_app_node_id index instead of scanning it. + That also means regexp_replace now runs over only the rows matching the + name pattern rather than the full indel row count, so the earlier + requirement to dedupe on protocol_app_node_id before applying + regexp_replace no longer applies here. --> + @@ -34,10 +44,10 @@ SELECT strain AS internal, strain AS term, strain AS display FROM ( SELECT DISTINCT regexp_replace(pan.name, '_Indel$', '') AS strain - FROM (SELECT DISTINCT protocol_app_node_id FROM apidb.indel) x - , study.protocolappnode pan - WHERE pan.protocol_app_node_id = x.protocol_app_node_id - AND pan.name LIKE '%\_Indel' + FROM study.protocolappnode pan + WHERE pan.name LIKE '%\_Indel' + AND EXISTS (SELECT 1 FROM apidb.indel i + WHERE i.protocol_app_node_id = pan.protocol_app_node_id) ) t ORDER BY strain ]]> diff --git a/Model/lib/wdk/model/questions/queries/strainSegmentQueries.xml b/Model/lib/wdk/model/questions/queries/strainSegmentQueries.xml index e23ec959f..5a3b265c9 100644 --- a/Model/lib/wdk/model/questions/queries/strainSegmentQueries.xml +++ b/Model/lib/wdk/model/questions/queries/strainSegmentQueries.xml @@ -2,21 +2,32 @@ - + Indel rows recorded on a sequence make strain and sequence consistent by + construction, so gate 4 needs no separate organism input to validate + against. It is scoped by na_sequence_id, never by strain name alone: + 126 strain names map to more than one organism. + + The apidb.organism join is also a gate, not just a project_id source: it + is an inner join, so a sequence whose taxon_id has no apidb.organism row + yields no record at all. That is intended - such a sequence sits outside + every project and so has no project_id to key a record on - and it does + not need to be a LEFT JOIN, since a NULL project_id would produce a + malformed primary key. + + project_id comes from apidb.organism.project_name because it is + unpartitioned and correct on every project, avoiding the per-project + query duplication that forces DynSpan to maintain UniDB-specific + variants. --> @@ -51,6 +62,11 @@ WHERE seg.ref_start >= 1 AND seg.ref_end >= seg.ref_start AND seg.ref_end <= seg.seq_length + -- The suffix is appended to the parameter (CONCAT($$strain$$, '_Indel')) + -- rather than stripped from the column (regexp_replace(pan.name, ...)), + -- so pan_named_ix (name, protocol_app_node_id, ...) can still be used as + -- an Index Only Scan; applying a function to pan.name instead would force + -- a scan of protocolappnode per row. AND EXISTS ( SELECT 1 FROM apidb.indel i From 0281aa9ad17eb1d32cdc5285c6f670bf0acfccf4 Mon Sep 17 00:00:00 2001 From: John Brestelli Date: Fri, 31 Jul 2026 13:16:56 -0400 Subject: [PATCH 13/42] Key strain offsets on the primary key to stop cross-sequence contamination One protocol_app_node_id spans every sequence of a strain, so grouping the offsets subquery on (strain, ref_start, ref_end) collapsed two segments on different sequences into one group and summed their shifts into both. Grouping and joining on source_id makes one-row-per-PK true by construction. Also clamps strain_length at zero for a segment falling inside a deletion, where the boundary rule legitimately inverts start and end, and expands WDK_ID_SQL once via a CTE. --- .../records/strainSegmentAttributeQueries.xml | 77 +++++++++++-------- 1 file changed, 44 insertions(+), 33 deletions(-) diff --git a/Model/lib/wdk/model/records/strainSegmentAttributeQueries.xml b/Model/lib/wdk/model/records/strainSegmentAttributeQueries.xml index 9ae0134b8..4025413ca 100644 --- a/Model/lib/wdk/model/records/strainSegmentAttributeQueries.xml +++ b/Model/lib/wdk/model/records/strainSegmentAttributeQueries.xml @@ -26,59 +26,70 @@ because the coordinate range is its own colon-delimited field. --> From b2deb6c362acd78e93998013733fb486e1489ab1 Mon Sep 17 00:00:00 2001 From: John Brestelli Date: Fri, 31 Jul 2026 13:18:30 -0400 Subject: [PATCH 14/42] Record the PK-keying and deletion-inversion decisions in the spec Adds sections 5.3.1 and 5.3.2 covering two findings from review of the attribute query. The offsets subquery must group and join on source_id, not on the coordinate triple. One protocol_app_node_id spans every sequence of a strain, so keying on (strain, ref_start, ref_end) conflated two segments on different sequences and summed their shifts into both, silently. Keying on the whole primary key makes one-row-per-PK true by construction. A segment lying inside a deletion legitimately inverts, because the boundary rule shifts the end but not the start. strain_start and strain_end report the true inverted values so the reporter can reject them; only strain_length is clamped. Co-Authored-By: Claude Opus 5 EOF --- ...2026-07-31-strain-segment-record-design.md | 50 +++++++++++++++++++ 1 file changed, 50 insertions(+) diff --git a/docs/superpowers/specs/2026-07-31-strain-segment-record-design.md b/docs/superpowers/specs/2026-07-31-strain-segment-record-design.md index ff0a12c9e..d5c66080a 100644 --- a/docs/superpowers/specs/2026-07-31-strain-segment-record-design.md +++ b/docs/superpowers/specs/2026-07-31-strain-segment-record-design.md @@ -306,6 +306,56 @@ shifts into one plausible-looking wrong answer with nothing to detect it. segment, so it shifts the end but not the start — hence `<` for start, `<=` for end. An off-by-one here is silent and yields sequence that looks correct. +### 5.3.1 Group and join on the primary key, not on coordinates + +An earlier implementation grouped the offsets subquery on `(strain, ref_start, ref_end)` +and joined on the same triple. **That silently corrupted every multi-record answer set.** +One `protocol_app_node_id` spans *all* of a strain's sequences (one Af293 node carries +indels on all 8 chromosomes), so two segments of the same strain on different sequences +collapsed into a single group whose `SUM` spanned both, and the join handed that conflated +row to both records. Measured on two 1-100000 segments of `NRZ-2016-071`, whose true +offsets are 12 (Chr1) and 14 (Chr2): both reported `strain_end = 100026`, i.e. 100000 + 26. +No error, no duplicate row, and `strain_seq_id` stayed correct — so the BED line pointed at +the right contig with another chromosome's indel budget applied to its coordinates. A +single-record test cannot see this; a reporter over a multi-segment result set is the +normal case. + +Adding `ref_seq` to the grouping would fix the symptom. Instead **group and join on +`source_id`** — the whole primary key — so "one row per input PK" holds by construction +rather than by an argument about which coordinate fields happen to discriminate. Verified +after the change: 100012 and 100014 respectively, and a third id differing only in strand +yields 3 rows rather than 6 (strand correctly does not discriminate, since offsets do not +depend on it). + +`i.protocol_app_node_id` stays in the `GROUP BY` alongside `source_id`, still absent from +the `SELECT` — that is the collision detector described above, and it is now the only +grouping column that is not the PK. + +`##WDK_ID_SQL##` is expanded **once**, into a `WITH ids AS (...)` CTE referenced twice. +Precedent: `transcriptAttributeQueries.xml:209`. Postgres materializes it (confirmed: a +single `CTE ids` node with two `CTE Scan` references), so the id set is scanned once and +the four `split_part` expressions exist in one place. + +### 5.3.2 A segment inside a deletion inverts, deliberately + +The boundary asymmetry means a deletion at `refStart` shifts the end but not the start, so +a short segment lying inside a deletion produces `strain_end < strain_start`. This is +reachable with ordinary input, not a torture case: `apidb.indel` has 180,998 rows with +`shift <= -20` and the ID query permits `start = end`. Real example — PK +`366.1:Pf3D7_10_v3:331757-331757:f`, a 1-bp segment on a `shift = -54` event, gives +`strain_start = 331658`, `strain_end = 331604`. + +That arithmetic is *correct*: the reference base does not exist in that strain, so the +range maps to nothing. So: + +- `strain_start` and `strain_end` report the true, possibly inverted values — **not** + clamped, because the reporter needs to see the inversion; +- `strain_length` is `GREATEST(..., 0)`, since a negative length is not a defensible + attribute value; +- the BED feature provider (§7) rejects `strain_end < strain_start` with an explicit + error. That is the right place for it — a malformed BED interval must fail loudly rather + than reach a FASTA lookup. + ## 6. Record class (`ApiCommonModel`) `StrainSegmentRecordClasses.StrainSegmentRecordClass`, From 889f0238d1325263a523fc65f7c4fd2ec8e110b2 Mon Sep 17 00:00:00 2001 From: John Brestelli Date: Fri, 31 Jul 2026 15:48:46 -0400 Subject: [PATCH 15/42] Make strain-offset derivation duplicate-safe; correct false underscore claim Follow-up review findings on StrainSegmentAttributes.Coords. Correctness: - Sum the offsets over a DISTINCT id set (new `segs` CTE) instead of the raw ##WDK_ID_SQL## output. A repeated source_id joined every indel row once per duplicate and GROUP BY source_id collapsed them into one group whose SUM was multiplied by the duplication factor -- silently. PK 366.1:Pf3D7_10_v3:331757-331757:f supplied twice gave offsets -198/-306 instead of -99/-153. Not reachable through today's ID query, but keying on the PK is meant to make one-row-per-PK true by construction, not by an argument about the input. Documented invariants (the code was right, the stated reasons were wrong): - strain_seq_id: the comment claimed strain names contain no underscore, so the first '_' ends the strain name. Re-measured: of the 6,119 strain names with indel data, 1,494 (24%) contain '_' (1_01_01, Af293_resequence2, China_LZCH-36, USGS_28834_1_NV) and 0 contain ':'. The concatenation is still correct -- it must match the dnaseq FASTA defline -- but the key is OPAQUE and must never be split back apart; consumers parse the strain out of the ':'-delimited PK. 0 actual key collisions today. - Collision detector: sharpened. All 126 duplicated strain names already have two protocol app nodes each carrying indel rows; what keeps the detector dormant is that no (name, na_sequence_id) pair is served by two nodes (0 today). That is the invariant the na_sequence_id join relies on. - StrainSegmentFeatureProvider rejecting the deletion inversion is retensed as a Task 6 requirement -- the class does not exist yet, so nothing rejects it today. Simplification: - offset_end drops its tautological CASE (the '<= ref_end' bound is already in the WHERE; verified 0 rows disagreeing) so the boundary asymmetry lives in exactly one place. offset_start keeps '<' -- that asymmetry is real. - strain_length is computed in an outer SELECT from the named strain_start / strain_end columns rather than repeating their offset arithmetic, so the length cannot drift from its own endpoints. - Hoist na_sequence_id/taxon_id into the `ids` CTE and drop the second dots.ExternalNaSequence join, halving the per-PK index scans and leaving one enforcement point for "unknown reference sequence yields no record". Verified against genomicsdb_rebuild01 (SELECT/EXPLAIN only): NRZ-2016-071 Chr1/Chr2 still 100012/100014; the 1-bp deletion PK still 331658/331604 length 0; three PKs differing only in strand still 3 rows; the same PK twice now yields 2 rows with single-copy offsets; EXPLAIN ANALYZE still uses indel_ix1 with `ids` materialized once. Co-Authored-By: Claude Opus 5 --- .../records/strainSegmentAttributeQueries.xml | 133 ++++++++++++------ ...2026-07-31-strain-segment-record-design.md | 119 +++++++++++----- 2 files changed, 176 insertions(+), 76 deletions(-) diff --git a/Model/lib/wdk/model/records/strainSegmentAttributeQueries.xml b/Model/lib/wdk/model/records/strainSegmentAttributeQueries.xml index 4025413ca..d300d737b 100644 --- a/Model/lib/wdk/model/records/strainSegmentAttributeQueries.xml +++ b/Model/lib/wdk/model/records/strainSegmentAttributeQueries.xml @@ -26,7 +26,10 @@ because the coordinate range is its own colon-delimited field. --> diff --git a/docs/superpowers/specs/2026-07-31-strain-segment-record-design.md b/docs/superpowers/specs/2026-07-31-strain-segment-record-design.md index d5c66080a..14e241a0c 100644 --- a/docs/superpowers/specs/2026-07-31-strain-segment-record-design.md +++ b/docs/superpowers/specs/2026-07-31-strain-segment-record-design.md @@ -71,13 +71,24 @@ Verified properties of the 6,119 distinct names appearing in `apidb.indel`: |---|---|---| | contain `:` | **0** | the colon-delimited PK grammar (§3) is unambiguous | | end in `_Indel` | **6,119 / 6,119** | the suffix strip is uniform; no special cases | -| contain any other `_` | **0** | `_` concatenation is unambiguous | +| contain any other `_` | **1,494** (24%) | `_` is **not** reversible — see §3.1 | | map to >1 organism | **126** (256 nodes) | strain name is **not** globally unique — see §5.2 | | `(name, na_sequence_id)` -> >1 node | **0** | sequence-scoped resolution is unambiguous *today* | -That last row is a **data-dependent invariant with no constraint enforcing it**. §5.2 -specifies a query shape that turns a future violation into an error rather than silent -corruption. +**Correction (re-measured 2026-07-31).** An earlier revision of this spec recorded "contain +any other `_`: 0" as verified fact. It is wrong: 1,494 of the 6,119 names contain an +underscore (`1_01_01`, `Af293_resequence2`, `China_LZCH-36`, `USGS_28834_1_NV`). The +`_` FASTA key is therefore a **one-way, opaque** key — correct to build, +never to split. Anything needing the strain or the sequence back takes it from the primary +key, where `:` delimits unambiguously (0 names contain `:`). See §3.1 for the consequence +for `StrainSegmentId`'s pattern, which relied on the false claim. + +The last row is a **data-dependent invariant with no constraint enforcing it**, and it is +the narrow one that matters: all 126 duplicated names *already* resolve to two protocol app +nodes that both carry indel rows today, so the ambiguity is live in the data — what keeps it +harmless is only that no `(name, na_sequence_id)` pair is served by two nodes. §5.2/§5.3 +specify a query shape that turns a future violation of *that* into an error rather than +silent corruption. ### `webready.genomicseqattributes_p` — reference sequences @@ -163,16 +174,27 @@ the range must never be parsed out of the whole string positionally. ^([^:_]+):([^:]+):(\d+)-(\d+):(f|r)$ ``` -Two details of that pattern are load-bearing: - -- Fields are `[^:]+`, not DynSpan's greedy `(.*)`, so an ID carrying an extra colon is - **rejected** rather than mis-parsed into a different segment. -- The **strain** group additionally excludes `_`, because the FASTA key is - `_` and reference sequence IDs legitimately contain underscores - (`Pf3D7_01_v3`). Without that exclusion, strain `A_B` + sequence `C` and strain `A` + - sequence `B_C` would both yield the key `A_B_C`. No current strain name contains an - underscore (0 of 6,119), so this is inert today and converts a future ambiguous key - into a loud parse failure. Group 2 stays `[^:]+`. +One detail of that pattern is load-bearing and one is **a defect that must be fixed**: + +- Load-bearing: fields are `[^:]+`, not DynSpan's greedy `(.*)`, so an ID carrying an extra + colon is **rejected** rather than mis-parsed into a different segment. +- **Defect — the strain group's `_` exclusion (`[^:_]+`) must be relaxed to `[^:]+`.** It was + added on the belief that no strain name contains an underscore ("0 of 6,119"), making the + exclusion inert insurance against an ambiguous `_` FASTA key. That belief + is false: **1,494 of the 6,119** strain names contain an underscore (§2, re-measured + 2026-07-31). As written, `parse()` therefore rejects roughly a quarter of all legitimate + strain segment IDs — e.g. `Af293_resequence2:Chr1_A_fumigatus_Af293:1-100:f`. This is not a + documentation nit; it is a live bug in Task 1's deliverable + (`ApiCommonWebsite/.../report/bed/util/StrainSegmentId.java:22`, plus whatever + `StrainSegmentIdTest` asserts about it) and blocks Task 6/7. + + The ambiguity the exclusion was meant to prevent is real (strain `A_B` + sequence `C` and + strain `A` + sequence `B_C` both yield the key `A_B_C`) but cannot be prevented by + narrowing the grammar, because both halves genuinely contain `_`. It is instead avoided by + **never reversing the key**: `strain_seq_id` is opaque output only, and every consumer that + needs the strain or the sequence reads them from the primary key's `:`-delimited fields + (§2, §5.3). Verified 0 actual key collisions in today's data; if one ever appears it is a + data problem to detect, not a grammar to tighten. Validation lives in the private constructor, not in `parse()`, so that any later from-parts factory cannot bypass it. It rejects `refStart < 1` (a 0 start would reach @@ -287,24 +309,33 @@ gate 3 of §5.1 rejects a strain with no data on the requested sequence. Emits `strain_seq_id`, `strain_start`, `strain_end`, `strain_length`, `organism`. `strain_seq_id` is `strain || '_' || refSeq` — pure concatenation, matching the FASTA -key (§2). Offsets in one index-assisted pass per `(strain, sequence)`, bounded by -`location <= ref_end`, using conditional aggregation rather than two correlated -subqueries: +key (§2), and **opaque**: build it, never split it (§3.1). Offsets in one index-assisted +pass per `(strain, sequence)`, bounded by `location <= ref_end`: ```sql -SUM(CASE WHEN i.location < seg.ref_start THEN i.shift ELSE 0 END) AS offset_start, -SUM(CASE WHEN i.location <= seg.ref_end THEN i.shift ELSE 0 END) AS offset_end +SUM(CASE WHEN i.location < seg.ref_start THEN i.shift ELSE 0 END) AS offset_start, +SUM(i.shift) AS offset_end ``` +Only the *start* offset needs conditional aggregation. The end bound +`i.location <= seg.ref_end` is in the `WHERE`, so every surviving row is in scope for the +end sum; an earlier revision wrote a mirrored +`CASE WHEN i.location <= seg.ref_end` whose `ELSE` branch was unreachable (verified 0 rows +disagreeing) and which made the boundary asymmetry appear to live in two places instead of +one. + **Resolve `protocol_app_node_id` via `(name, na_sequence_id)` and `GROUP BY` the -resolved node — never join on name alone.** 126 strain names map to more than one -organism (§2). Grouping by node makes a future collision produce two rows per primary -key, which WDK rejects with an error. Joining on name alone would sum two strains' -shifts into one plausible-looking wrong answer with nothing to detect it. +resolved node — never join on name alone.** All 126 duplicated strain names (§2) *already* +have two protocol app nodes each carrying indel rows today, so the collision is live in the +data; what keeps it harmless is the narrower invariant the `i.na_sequence_id = +ens.na_sequence_id` join depends on — **no `(name, na_sequence_id)` pair is served by two +nodes (0 today)**. Grouping by node makes a violation of that produce two rows per primary +key, which WDK rejects with an error. Joining on name alone would sum two strains' shifts +into one plausible-looking wrong answer with nothing to detect it. **Boundary rule (to be QA'd, per §9):** an indel exactly at `refStart` lies inside the -segment, so it shifts the end but not the start — hence `<` for start, `<=` for end. An -off-by-one here is silent and yields sequence that looks correct. +segment, so it shifts the end but not the start — hence `<` for start and `<=` (as the +`WHERE` bound) for end. An off-by-one here is silent and yields sequence that looks correct. ### 5.3.1 Group and join on the primary key, not on coordinates @@ -331,10 +362,31 @@ depend on it). the `SELECT` — that is the collision detector described above, and it is now the only grouping column that is not the PK. -`##WDK_ID_SQL##` is expanded **once**, into a `WITH ids AS (...)` CTE referenced twice. -Precedent: `transcriptAttributeQueries.xml:209`. Postgres materializes it (confirmed: a -single `CTE ids` node with two `CTE Scan` references), so the id set is scanned once and -the four `split_part` expressions exist in one place. +`##WDK_ID_SQL##` is expanded **once**, into a `WITH parsed AS (...)` CTE; `ids` adds the +resolved `dots.ExternalNaSequence` row to it and is referenced twice. Precedent: +`transcriptAttributeQueries.xml:209`. Postgres materializes `ids` (confirmed: a single +`CTE ids` node with two `CTE Scan` references), so the id set is scanned once and the four +`split_part` expressions exist in one place. + +Three further structural rules, each guarding a silent-wrong-answer mode: + +- **`ids` resolves the reference sequence, and is the *only* place that does.** It therefore + *filters* as well as projects — an unknown `ref_seq` drops the row here, which is exactly + the "unknown sequence yields no record" rule. The offsets subquery originally repeated the + same `dots.ExternalNaSequence` lookup (a second index scan per PK, and the same rule + enforced in two places); it now consumes `ids.na_sequence_id`. +- **The offsets are summed over a `DISTINCT` id set (`segs`), not the raw one.** Scanning the + raw set makes the sums proportional to input multiplicity: a repeated `source_id` in + `##WDK_ID_SQL##` joins each indel row once per duplicate and `GROUP BY source_id` collapses + them into one group whose `SUM` is multiplied. Demonstrated before the fix — PK + `366.1:Pf3D7_10_v3:331757-331757:f` supplied twice yielded offsets −198/−306 instead of + −99/−153, silently. Not reachable through today's ID query, but keying on the PK is + supposed to make one-row-per-PK true *by construction*, not by an argument about the input. + Regression test: the same PK twice must yield 2 rows with the single-copy offsets. +- **`strain_length` is computed in an outer `SELECT` from the named `strain_start` / + `strain_end` columns**, because a `SELECT` cannot reference its own aliases and the offset + arithmetic would otherwise be written twice. A partial edit to one copy would yield a + length that disagrees with its own start and end. ### 5.3.2 A segment inside a deletion inverts, deliberately @@ -352,9 +404,12 @@ range maps to nothing. So: clamped, because the reporter needs to see the inversion; - `strain_length` is `GREATEST(..., 0)`, since a negative length is not a defensible attribute value; -- the BED feature provider (§7) rejects `strain_end < strain_start` with an explicit - error. That is the right place for it — a malformed BED interval must fail loudly rather - than reach a FASTA lookup. +- `StrainSegmentFeatureProvider` (§7) **MUST** reject `strain_end < strain_start` with an + explicit error. That is the right place for it — a malformed BED interval must fail loudly + rather than reach a FASTA lookup. **Not yet implemented:** that class is Task 6 and does not + exist as of the attribute-query commit, so today nothing rejects the inversion and an + inverted segment reaches consumers unflagged. This is a required deliverable of Task 6, not + an existing property of the system. ## 6. Record class (`ApiCommonModel`) From 8396af32986d19b2cdee73621586bc077ee9d432 Mon Sep 17 00:00:00 2001 From: John Brestelli Date: Fri, 31 Jul 2026 15:59:26 -0400 Subject: [PATCH 16/42] Correct the spec's attribute list and the node-count claims MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two documentation defects found by review, both of which would have misled the next reader: - §5.3 listed only the five derived columns and §6 said "exactly those §5.3 emits", which read literally forbade the ref_* attributes that §7's defline consumes. The query has always emitted eleven columns; the spec understated it. Now names all eleven and says why the primary-key fields are passed through rather than re-parsed. - "126 strain names resolve to two protocol app nodes" is imprecise: 122 resolve to two and 4 to three, which is where §2's count of 256 nodes comes from. Corrected in the spec (twice) and in the collision-detector comment. The invariant that keeps the detector dormant is unchanged: no (name, na_sequence_id) pair is served by more than one node. --- .../records/strainSegmentAttributeQueries.xml | 3 ++- ...2026-07-31-strain-segment-record-design.md | 23 +++++++++++++------ 2 files changed, 18 insertions(+), 8 deletions(-) diff --git a/Model/lib/wdk/model/records/strainSegmentAttributeQueries.xml b/Model/lib/wdk/model/records/strainSegmentAttributeQueries.xml index d300d737b..b9b658259 100644 --- a/Model/lib/wdk/model/records/strainSegmentAttributeQueries.xml +++ b/Model/lib/wdk/model/records/strainSegmentAttributeQueries.xml @@ -90,7 +90,8 @@ -- name resolving to two protocol app nodes ON THE SAME SEQUENCE yields -- DUPLICATE rows per PK and a loud WDK error, rather than silently summing -- two strains' shifts. Measured today: 126 strain names each resolve to - -- two protocol app nodes that BOTH already carry indel rows, so the + -- two OR MORE protocol app nodes (122 to two, 4 to three; 256 nodes in + -- all) that ALL already carry indel rows, so the -- ambiguity exists in the data right now. What keeps this detector dormant -- is the narrower invariant the join on na_sequence_id relies on -- no -- (strain name, na_sequence_id) pair is served by more than one node diff --git a/docs/superpowers/specs/2026-07-31-strain-segment-record-design.md b/docs/superpowers/specs/2026-07-31-strain-segment-record-design.md index 14e241a0c..c87f426bf 100644 --- a/docs/superpowers/specs/2026-07-31-strain-segment-record-design.md +++ b/docs/superpowers/specs/2026-07-31-strain-segment-record-design.md @@ -84,9 +84,10 @@ key, where `:` delimits unambiguously (0 names contain `:`). See §3.1 for the c for `StrainSegmentId`'s pattern, which relied on the false claim. The last row is a **data-dependent invariant with no constraint enforcing it**, and it is -the narrow one that matters: all 126 duplicated names *already* resolve to two protocol app -nodes that both carry indel rows today, so the ambiguity is live in the data — what keeps it -harmless is only that no `(name, na_sequence_id)` pair is served by two nodes. §5.2/§5.3 +the narrow one that matters: all 126 duplicated names *already* resolve to two or more +protocol app nodes (122 to two, 4 to three) that all carry indel rows today, so the ambiguity +is live in the data — what keeps it harmless is only that no `(name, na_sequence_id)` pair is +served by more than one node. §5.2/§5.3 specify a query shape that turns a future violation of *that* into an error rather than silent corruption. @@ -306,7 +307,11 @@ gate 3 of §5.1 rejects a strain with no data on the requested sequence. ### 5.3 Attribute query: `StrainSegmentAttributes.Coords` -Emits `strain_seq_id`, `strain_start`, `strain_end`, `strain_length`, `organism`. +Emits eleven columns. Derived: `strain_seq_id`, `strain_start`, `strain_end`, +`strain_length`, `organism`. Passed through from the primary key so that consumers never +re-parse it: `source_id`, `project_id`, `strain`, `ref_seq`, `ref_start`, `ref_end` — §7's +defline needs the strain and the reference range, and decomposing the PK in exactly one +place is the whole point of §3.1. `strain_seq_id` is `strain || '_' || refSeq` — pure concatenation, matching the FASTA key (§2), and **opaque**: build it, never split it (§3.1). Offsets in one index-assisted @@ -326,7 +331,8 @@ one. **Resolve `protocol_app_node_id` via `(name, na_sequence_id)` and `GROUP BY` the resolved node — never join on name alone.** All 126 duplicated strain names (§2) *already* -have two protocol app nodes each carrying indel rows today, so the collision is live in the +have two or more protocol app nodes (122 to two, 4 to three), all carrying indel rows +today, so the collision is live in the data; what keeps it harmless is the narrower invariant the `i.na_sequence_id = ens.na_sequence_id` join depends on — **no `(name, na_sequence_id)` pair is served by two nodes (0 today)**. Grouping by node makes a violation of that produce two rows per primary @@ -416,8 +422,11 @@ range maps to nothing. So: `StrainSegmentRecordClasses.StrainSegmentRecordClass`, `urlName="strain-genomic-segment"`, `doNotTest="true"`. -Reporters: **`bed` only** (see §8). Attributes: exactly those §5.3 emits, plus the -`idAttribute`. No tables, no summary view, no text attributes for display. +Reporters: **`bed` only** (see §8). Attributes: a `columnAttribute` for each of the nine +non-primary-key columns §5.3 emits — the passed-through `strain`, `ref_seq`, `ref_start`, +`ref_end` included, since §7's defline consumes them — plus the `idAttribute` over the +`source_id`/`project_id` primary key. No tables, no summary view, no text attributes for +display. Register the new files in `Model/lib/wdk/apiCommonModel.xml`, alongside the existing span imports at lines 419-423. From 0787ef116a60076f70ce854270384b4dec11b043 Mon Sep 17 00:00:00 2001 From: John Brestelli Date: Fri, 31 Jul 2026 16:03:58 -0400 Subject: [PATCH 17/42] Refuse to mint a primary key for a colon-bearing reference sequence ':' is the only delimiter in the strain segment primary key, so a reference sequence whose source_id contains one produces an ID that the attribute query mis-parses and then dies on -- 'ncRNA'::integer aborts the ENTIRE attribute query, taking out every row on the page rather than just the offending record. 10,704 dots.ExternalNaSequence source_ids do contain ':' (transcript features such as bld68_Tb927.1.05:mRNA). None of the 20,823 indel-bearing sequences do, so this is unreachable today -- which is the reason to gate it rather than a reason not to: the grammar should not depend on which features happen to get indel-called. Verified the gate refuses 0 indel-bearing sequences and that all 8 Af293 chromosomes still pass. Also corrects three comments the review found under-evidenced or unverifiable: - the colon-unambiguity claim covered "the strain or the sequence" but measured only strain names; now measures both, and points at the gate - the FASTA defline was quoted as '{sample}_{chrom_name}', which reads as a verbatim f-string; now matches the spec's notation and cites makeConsensusFastaFromVcfAndBed.py:233 - documents the sres.TaxonName inner join as the second silent row-drop point, and the cross-query inconsistency that the ID query resolves organism via apidb.organism while this one uses TaxonName --- .../queries/strainSegmentQueries.xml | 11 ++++++++++ .../records/strainSegmentAttributeQueries.xml | 20 ++++++++++++++++--- ...2026-07-31-strain-segment-record-design.md | 15 ++++++++++++-- 3 files changed, 41 insertions(+), 5 deletions(-) diff --git a/Model/lib/wdk/model/questions/queries/strainSegmentQueries.xml b/Model/lib/wdk/model/questions/queries/strainSegmentQueries.xml index 5a3b265c9..e379497e1 100644 --- a/Model/lib/wdk/model/questions/queries/strainSegmentQueries.xml +++ b/Model/lib/wdk/model/questions/queries/strainSegmentQueries.xml @@ -62,6 +62,17 @@ WHERE seg.ref_start >= 1 AND seg.ref_end >= seg.ref_start AND seg.ref_end <= seg.seq_length + -- ':' is the ONLY delimiter in the primary key this query mints, so a + -- reference sequence whose source_id contains one would produce an + -- unparseable ID -- StrainSegmentAttributes.Coords would split it at the + -- embedded colon and then abort the ENTIRE attribute query (every row on + -- the page, not just the offender) casting 'ncRNA'::integer. Refusing to + -- mint such an ID here makes the grammar sound BY CONSTRUCTION instead of + -- by a property of what happens to be loaded: 10,704 dots.ExternalNaSequence + -- source_ids do contain ':' (e.g. bld68_Tb927.1.05:mRNA), and today none of + -- the 20,823 indel-bearing ones do -- but that is an accident of which + -- features get indel-called, not an invariant anyone enforces. + AND seg.ref_seq NOT LIKE '%:%' -- The suffix is appended to the parameter (CONCAT($$strain$$, '_Indel')) -- rather than stripped from the column (regexp_replace(pan.name, ...)), -- so pan_named_ix (name, protocol_app_node_id, ...) can still be used as diff --git a/Model/lib/wdk/model/records/strainSegmentAttributeQueries.xml b/Model/lib/wdk/model/records/strainSegmentAttributeQueries.xml index b9b658259..c771c7386 100644 --- a/Model/lib/wdk/model/records/strainSegmentAttributeQueries.xml +++ b/Model/lib/wdk/model/records/strainSegmentAttributeQueries.xml @@ -119,19 +119,33 @@ , ids.ref_seq , ids.ref_start , ids.ref_end - -- OPAQUE key -- must match the dnaseq FASTA defline '{sample}_{chrom_name}' - -- (spec 2), and must NEVER be split back apart. The '_' is not a usable + -- OPAQUE key -- must match the dnaseq FASTA defline _, + -- written by dnaseq-nextflow bin/makeConsensusFastaFromVcfAndBed.py:233 + -- (spec 2; that script is not in this checkout), and must NEVER be + -- split back apart. The '_' is not a usable -- delimiter: measured over the 6,119 strain names carrying indel data, -- 1,494 (24%) themselves contain '_' (1_01_01, Af293_resequence2, -- China_LZCH-36, USGS_28834_1_NV), so "the first '_' ends the strain name" -- is false for roughly a quarter of strains. A consumer needing the -- strain or the sequence takes it from the primary key, where ':' - -- delimits unambiguously (0 of 6,119 strain names contain ':'). + -- delimits unambiguously: 0 of 6,119 strain names contain ':', and the + -- ID query refuses to mint an ID for a reference sequence containing + -- one (10,704 dots.ExternalNaSequence source_ids do, though none of + -- the 20,823 indel-bearing ones -- see that query's ref_seq gate). , ids.strain || '_' || ids.ref_seq AS strain_seq_id , ids.ref_start + COALESCE(off.offset_start, 0) AS strain_start , ids.ref_end + COALESCE(off.offset_end, 0) AS strain_end , tn.name AS organism FROM ids + -- Second silent row-drop point (the ids CTE is the other): an inner join, + -- so a sequence whose taxon has no 'scientific name' row disappears with + -- no record and no error. 0 affected today (0 sequences with a NULL + -- taxon, 0 taxa missing a scientific name). Note the cross-query + -- inconsistency this leaves: the ID query decides "does this sequence + -- have an organism" via apidb.organism, this one via sres.TaxonName. + -- They agree on all current data; nothing enforces that they keep + -- agreeing, so a divergence would show up as records that pass the ID + -- query and then vanish from the page. JOIN sres.TaxonName tn ON tn.taxon_id = ids.taxon_id AND tn.name_class = 'scientific name' LEFT JOIN offsets off ON off.source_id = ids.source_id diff --git a/docs/superpowers/specs/2026-07-31-strain-segment-record-design.md b/docs/superpowers/specs/2026-07-31-strain-segment-record-design.md index c87f426bf..67b839215 100644 --- a/docs/superpowers/specs/2026-07-31-strain-segment-record-design.md +++ b/docs/superpowers/specs/2026-07-31-strain-segment-record-design.md @@ -252,13 +252,24 @@ Dropping it removes three problems at once: > `includeProjects`/`excludeProjects`, which this record may not have. Not taking an > organism at all sidesteps the whole question. -Three gates, all as filters so bad input yields **zero records** rather than a broken +Four gates, all as filters so bad input yields **zero records** rather than a broken download: 1. the reference sequence exists — `dots.ExternalNaSequence.source_id = $$sequenceId$$`; 2. `1 <= refStart <= refEnd <= ens.length`; 3. the strain has indel data on **this** sequence — an `EXISTS` over `apidb.indel` joined to - `study.protocolappnode`, matched on `na_sequence_id`. + `study.protocolappnode`, matched on `na_sequence_id`; +4. the reference sequence ID contains no `':'` — `seg.ref_seq NOT LIKE '%:%'`. + +Gate 4 exists because `':'` is the only delimiter in the minted primary key, so a +colon-bearing `source_id` yields an ID that §5.3 mis-parses and then dies on +(`'ncRNA'::integer` aborts the *entire* attribute query, every row on the page). 10,704 +`dots.ExternalNaSequence` source_ids do contain a colon — `bld68_Tb927.1.05:mRNA` and +similar transcript-level features — while **0** of the 20,823 indel-bearing sequences do. +That makes the gate unreachable today, which is exactly why it is worth having: the +grammar's soundness should not rest on which features happen to get indel-called. Verified +the gate costs nothing real — it refuses 0 indel-bearing sequences and all 8 Af293 +chromosomes still pass. Gate 3 subsumes the organism check the earlier draft did explicitly: if the strain has indel rows on that sequence, strain and sequence are consistent **by construction**. That is a From c062fc5a040e97e5b04085d81b34306d427e18be Mon Sep 17 00:00:00 2001 From: John Brestelli Date: Fri, 31 Jul 2026 16:12:19 -0400 Subject: [PATCH 18/42] Add strain segment record class and question, and import the model files The record class is deliberately bare: primary key, idAttribute, and the nine non-PK columnAttributes from StrainSegmentAttributes.Coords. No tables, no summary view, no display attributes, so a record-page request fails rather than rendering an empty page. No includeProjects/excludeProjects anywhere -- unlike DynSpanRecordClass, project_id comes from the data, so no per-project fork is needed. The bed reporter element is NOT included yet: ReporterRef.resolveReferences does a Class.forName on the implementation at model-load time and throws, so the element cannot land before BedStrainSegmentReporter does. Task 6 adds both. No individuals.txt row, which is what keeps the search out of the category tree while leaving it addressable by name through the service API. wb model therefore suffices; wb ontology is not required. Co-Authored-By: Claude Opus 5 --- Model/lib/wdk/apiCommonModel.xml | 7 +++ .../questions/strainSegmentQuestions.xml | 28 ++++++++++ .../wdk/model/records/strainSegmentRecord.xml | 55 +++++++++++++++++++ 3 files changed, 90 insertions(+) create mode 100644 Model/lib/wdk/model/questions/strainSegmentQuestions.xml create mode 100644 Model/lib/wdk/model/records/strainSegmentRecord.xml diff --git a/Model/lib/wdk/apiCommonModel.xml b/Model/lib/wdk/apiCommonModel.xml index 3c098ce9f..79eb71c52 100644 --- a/Model/lib/wdk/apiCommonModel.xml +++ b/Model/lib/wdk/apiCommonModel.xml @@ -423,6 +423,13 @@ + + + + + + + + + + + + + Given a genomic location in reference coordinates and a strain, return the + equivalent segment in that strain's consensus sequence coordinates. + + + + + + + diff --git a/Model/lib/wdk/model/records/strainSegmentRecord.xml b/Model/lib/wdk/model/records/strainSegmentRecord.xml new file mode 100644 index 000000000..168a28abc --- /dev/null +++ b/Model/lib/wdk/model/records/strainSegmentRecord.xml @@ -0,0 +1,55 @@ + + + + + + + + + source_id + project_id + + + + + + + + + + + + + + + + + + + + + + + + + From d41cfb6ae7700c3a96157779a7bb6f55d1292d71 Mon Sep 17 00:00:00 2001 From: John Brestelli Date: Fri, 31 Jul 2026 16:20:17 -0400 Subject: [PATCH 19/42] Fix plan and spec defects that would have broken Tasks 6 and 7 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Task 5's spec review found two plan defects with real downstream cost, plus staleness that had already caused one live bug. Task 6 could not have succeeded as written: - Its Files list named only the two ApiCommonWebsite Java files, with no step registering the element on the record class. Task 5 had to omit that element -- WDK's ReporterRef.resolveReferences calls Class.forName at model-load time, so registering a class that does not exist yet hard-fails the build -- and the deferral note lived only in the XML and a commit message, neither on a Task 6 implementer's path. Without the registration the BED download is simply not offered, with no error; Task 7 would have failed on an unknown format. Task 6 now spans both repos and has explicit register + rebuild steps. - Task 6 now also states that rejecting the inverted interval is a required deliverable, not a nicety. The attribute query deliberately leaves strain_start/strain_end unclamped so a segment inside a deletion inverts; nothing upstream rejects it, so if the provider does not, an inverted interval reaches a FASTA lookup silently. Removed the stale organismSinglePick expectations from Task 5 Step 5 and Task 7 Step 3. There is no organism param; WDK rejects unknown parameters, so Task 7 would have failed on its first call. Corrected the false measurement at the root of the Task 1 regex bug. Task 2's preamble asserted that no strain name contains an underscore; 1,494 of 6,119 (24%) do. That claim propagated into the PK grammar as [^:_]+ and silently rejected a quarter of legitimate primary keys. The plan and spec now record the real numbers, the fix, and why narrowing the grammar could never have worked: reference sequence IDs contain underscores too, so _ is not reversible at all. Also: Task 5 Step 5 claimed -showQuery would print the attribute query with ##WDK_ID_SQL## expanded. It does not, and that is correct -- run standalone through QueryTester there is no answer to supply the ID SQL. The step now says what it actually proves. And §3's project_id provenance named webready.GenomicSeqAttributes_p; the implementation uses apidb.organism.project_name. --- .../plans/2026-07-31-strain-segment-record.md | 105 +++++++++++++++--- ...2026-07-31-strain-segment-record-design.md | 41 ++++--- 2 files changed, 112 insertions(+), 34 deletions(-) diff --git a/docs/superpowers/plans/2026-07-31-strain-segment-record.md b/docs/superpowers/plans/2026-07-31-strain-segment-record.md index 063f4a7fb..fc26a9042 100644 --- a/docs/superpowers/plans/2026-07-31-strain-segment-record.md +++ b/docs/superpowers/plans/2026-07-31-strain-segment-record.md @@ -50,10 +50,10 @@ bash ~/workspaces/agentic-veupath-dev/bin/veup-logs.sh fungidb since t1 | File | Responsibility | |---|---| -| `model/questions/params/strainSegmentParams.xml` | *new* — the `strain` flat-vocab param and its organism-dependent vocabulary query | +| `model/questions/params/strainSegmentParams.xml` | *new* — the `strain` flat-vocab param and its **global** vocabulary query (not organism-dependent; there is no organism param — spec §5.1.1) | | `model/questions/queries/strainSegmentQueries.xml` | *new* — the ID query; all four validation gates live here | | `model/records/strainSegmentAttributeQueries.xml` | *new* — the reference-to-strain coordinate conversion | -| `model/records/strainSegmentRecord.xml` | *new* — record class, attributes, BED reporter registration | +| `model/records/strainSegmentRecord.xml` | *new* — record class and attributes. The BED `` element is added in **Task 6**, not Task 5: WDK's `ReporterRef.resolveReferences` does `Class.forName` at model-load time, so registering it before the Java class exists hard-fails the build | | `model/questions/strainSegmentQuestions.xml` | *new* — the question that binds query to record class | | `apiCommonModel.xml` | *modify* — five `` lines near the existing span imports at 419-423 | @@ -72,14 +72,19 @@ One responsibility each. The grammar class is the only piece with no WDK depende ## Task 1: The PK grammar class -> **COMPLETE — and amended after code review.** Commits `19defeba2` (as written below) -> and `3ead3adac` (review fixes). The code block in Step 3 is what was *planned*; the -> shipped version differs in three ways, all worth knowing if you touch this class: -> the regex is `^([^:_]+):([^:]+):(\d+)-(\d+):(f|r)$` (strain excludes `_`, because -> `_` would otherwise be ambiguous — sequence IDs contain underscores); -> validation lives in the private constructor rather than `parse()`, and adds a -> `refStart < 1` check; and both `parseInt` calls rethrow with the full ID in the -> message. 14 tests, not 12. See spec §3.1 for the reasoning. +> **COMPLETE — amended twice after code review.** Commits `19defeba2` (as written below), +> `3ead3adac` (review fixes), `65126443a` (bug fix). The code block in Step 3 is what was +> *planned*; the shipped version differs, all worth knowing if you touch this class: +> the regex is `^([^:]+):([^:]+):(\d+)-(\d+):(f|r)$` — every field excludes only `':'`, +> the sole delimiter; validation lives in the private constructor rather than `parse()`, +> and adds a `refStart < 1` check; and both `parseInt` calls rethrow with the full ID in +> the message. 20 tests, not 12. See spec §3.1 for the reasoning. +> +> **The strain group was briefly `[^:_]+` and that was a live bug**, rejecting the 1,494 of +> 6,119 strain names (24%) that contain an underscore. It came from a false "no strain name +> contains an underscore" measurement asserted in Task 2's preamble. Do not reintroduce it: +> narrowing the grammar cannot make `_` reversible anyway, because reference +> sequence IDs contain underscores too. The key is opaque; parse the PK instead. The `ApiCommonWebsite/Model` module has JUnit 4 on its classpath but no `src/test/java` tree yet. You are creating it. Standard Maven layout means Surefire picks it up with no POM change. @@ -309,7 +314,16 @@ bash ~/workspaces/agentic-veupath-dev/bin/veup-git-sync.sh fungidb ## Task 2: Strain vocabulary param -The strain list must come from `apidb.indel` (the requirement) and be scoped to the selected organism. There is no strain column on `apidb.indel`; strain is `study.protocolappnode.name` minus a `_Indel` suffix. All 6,119 names carry that suffix, none contains a colon, and none contains any other underscore — so both the strip and the later `_` concatenation are unambiguous. +The strain list must come from `apidb.indel` (the requirement). There is no strain column on `apidb.indel`; strain is `study.protocolappnode.name` minus a `_Indel` suffix. All 6,119 names carry that suffix and **none contains a colon**, which is what makes the primary key parseable — `':'` is its only delimiter. + +> **CORRECTION — an earlier draft of this line claimed "none contains any other underscore". +> That is FALSE: 1,494 of the 6,119 names (24%) contain an underscore** — `1_01_01`, +> `Af293_resequence2`, `China_LZCH-36`, `USGS_28834_1_NV`. The `_Indel` strip is still +> unambiguous (it is anchored to the end), but the `_` concatenation is +> **not reversible** and must be treated as an opaque key. This bad fact propagated into +> Task 1's regex as `[^:_]+` for the strain group, which silently rejected ~24% of +> legitimate primary keys until it was caught in review. Consumers needing the strain parse +> it from the primary key, where `':'` delimits unambiguously. > **REVISED after review — there is no organism param.** An earlier draft made this > vocabulary depend on an organism param and scope itself with `org_abbrev`. That was @@ -888,9 +902,16 @@ ssh cedar 'bash -lc "source /var/www/jbrestel.fungidb.org/etc/setenv && \ wdkQuery -model FungiDB -query StrainSegmentId.StrainSegmentsByRefSegment -showParams"' ``` -Expected: the attribute query prints with `##WDK_ID_SQL##` expanded; the ID query lists -six params (`organismSinglePick`, `strain`, `sequenceId`, `start_point`, -`end_point_segment`, `sequence_strand`). +Expected: the ID query lists **five** params — `strain`, `sequenceId`, `start_point`, +`end_point_segment`, `sequence_strand`. There is **no** organism param (spec §5.1.1); if +you see one, something is wrong. + +The attribute query prints wrapped as `SELECT o.* FROM (…) o` with `##WDK_ID_SQL##` +**still literal, NOT expanded** — and that is correct, not a failure. Run standalone +through `QueryTester` there is no answer or step to supply the ID SQL, so nothing +substitutes the macro (the log also reports `params: [ ]`). Substitution happens in the +answer-value layer at request time. What this step proves is that the query is +registered, parses, and resolves — *executing* it against real IDs is Task 7's job. - [ ] **Step 6: Commit** @@ -923,6 +944,21 @@ Two BED columns, two very different constraints: **Files:** - Create: `ApiCommonWebsite/Model/src/main/java/org/apidb/apicommon/model/report/bed/feature/StrainSegmentFeatureProvider.java` - Create: `ApiCommonWebsite/Model/src/main/java/org/apidb/apicommon/model/report/bed/BedStrainSegmentReporter.java` +- Modify: `ApiCommonModel/Model/lib/wdk/model/records/strainSegmentRecord.xml` — register the + reporter (Task 5 deliberately left this out; see Step 4 below). **Two repos, one task.** + +> **The registration in Step 4 is not optional bookkeeping — without it the BED download +> simply is not offered, with no error anywhere.** Task 7's download call would fail with an +> unknown-format error and the cause would not be obvious. Do not consider this task done +> with only the Java side written. + +**This task must also reject the inverted-interval case.** `StrainSegmentAttributes.Coords` +deliberately leaves `strain_start`/`strain_end` unclamped so that a segment lying inside a +deletion reports `strain_end < strain_start` (only `strain_length` is clamped to 0 — spec +§5.3.2). Nothing upstream rejects it, so the provider **must**: an inverted interval that +reaches a FASTA lookup is a silent wrong answer. Verified live example to test against: +PK `366.1:Pf3D7_10_v3:331757-331757:f` yields `strain_start 331658`, `strain_end 331604`, +`strain_length 0`. - [ ] **Step 1: Create the feature provider** @@ -1063,13 +1099,49 @@ Expected: `Tests run: 12, Failures: 0` and no compilation errors. typo in an attribute name surfaces on first use, not at compile time. That is what Task 7 Step 2 checks. -- [ ] **Step 4: Commit** +- [ ] **Step 4: Register the reporter on the record class** + +Task 5 could not do this: WDK's `ReporterRef.resolveReferences` +(`WDK/Model/src/main/java/org/gusdb/wdk/model/report/ReporterRef.java:213-224`) calls +`Class.forName(getImplementation())` at model-load time and throws +`WdkModelException("… cannot be found.")` on `ClassNotFoundException`. Registering a +reporter whose class did not yet exist would have hard-failed the build. Now that +`BedStrainSegmentReporter` exists, add it. + +In `ApiCommonModel/Model/lib/wdk/model/records/strainSegmentRecord.xml`, replace the +Task-6 placeholder comment inside `` with: + +```xml + +``` + +`name="bed"` is the string Task 7 passes as `reportName`, so it must match exactly. + +- [ ] **Step 5: Rebuild so the model picks up the registration** + +```bash +bash ~/workspaces/agentic-veupath-dev/bin/veup-build.sh fungidb wb model +``` + +Expected: build succeeds. If it fails with "Implementation class for reporter 'bed' … +cannot be found", the Java class did not compile into the deployed webapp — fix that +before continuing, since Task 7 cannot pass without it. + +- [ ] **Step 6: Commit — both repos** ```bash cd ~/workspaces/fungidb/ApiCommonWebsite git add Model/src/main/java/org/apidb/apicommon/model/report/bed/feature/StrainSegmentFeatureProvider.java \ Model/src/main/java/org/apidb/apicommon/model/report/bed/BedStrainSegmentReporter.java git commit -m "Add BED reporter and feature provider for strain genomic segments" + +cd ~/workspaces/fungidb/ApiCommonModel +git add Model/lib/wdk/model/records/strainSegmentRecord.xml +git commit -m "Register the BED reporter on the strain segment record class" + bash ~/workspaces/agentic-veupath-dev/bin/veup-git-sync.sh fungidb ``` @@ -1111,7 +1183,8 @@ const r = await fetch('/a/service/answer', { body: JSON.stringify({ searchName: 'StrainSegmentsByRefSegment', searchConfig: {parameters: { - organismSinglePick: 'afumAf293', + // NO organism param -- the search does not declare one (spec §5.1.1) and WDK + // rejects unknown parameters. Five params exactly. strain: '', // from Task 3 Step 2 sequenceId: '', // from Task 3 Step 2 start_point: '100', diff --git a/docs/superpowers/specs/2026-07-31-strain-segment-record-design.md b/docs/superpowers/specs/2026-07-31-strain-segment-record-design.md index 67b839215..f632d61d4 100644 --- a/docs/superpowers/specs/2026-07-31-strain-segment-record-design.md +++ b/docs/superpowers/specs/2026-07-31-strain-segment-record-design.md @@ -136,9 +136,10 @@ included. This is a deliberate departure from DynSpan, which carries an `excludeProjects="UniDB"` PK column *and* duplicated UniDB `` variants (`dynSpanAttributeQueries.xml:16-33`). That fork exists because DynSpan derives `project_id` from `@PROJECT_ID@`, which is meaningless -on the portal. This record instead takes `project_id` from the data -(`webready.GenomicSeqAttributes_p.project_id`), so it is populated correctly on every -project including UniDB and needs no fork. One query, one PK, no variants. +on the portal. This record instead takes `project_id` from the data — `apidb.organism.project_name`, +joined on `taxon_id` (an earlier draft of this line named `webready.GenomicSeqAttributes_p.project_id`; +§5.1.1 explains why the implementation uses the unpartitioned `apidb.organism` instead) — so it is +populated correctly on every project including UniDB and needs no fork. One query, one PK, no variants. Note this is separate from the requirement to add UniDB to the **existing** genomic-segment `includeProjects` lists where absent — that concerns pre-existing @@ -169,25 +170,29 @@ feature provider. On the SQL side use `split_part(source_id, ':', N)`, not posit contain `-` (`A17-48H-7`), which is safe only because the range is its own colon field; the range must never be parsed out of the whole string positionally. -**As implemented** (commits `19defeba2`, `3ead3adac`): +**As implemented** (commits `19defeba2`, `3ead3adac`, corrected by `65126443a`): ``` -^([^:_]+):([^:]+):(\d+)-(\d+):(f|r)$ +^([^:]+):([^:]+):(\d+)-(\d+):(f|r)$ ``` -One detail of that pattern is load-bearing and one is **a defect that must be fixed**: - -- Load-bearing: fields are `[^:]+`, not DynSpan's greedy `(.*)`, so an ID carrying an extra - colon is **rejected** rather than mis-parsed into a different segment. -- **Defect — the strain group's `_` exclusion (`[^:_]+`) must be relaxed to `[^:]+`.** It was - added on the belief that no strain name contains an underscore ("0 of 6,119"), making the - exclusion inert insurance against an ambiguous `_` FASTA key. That belief - is false: **1,494 of the 6,119** strain names contain an underscore (§2, re-measured - 2026-07-31). As written, `parse()` therefore rejects roughly a quarter of all legitimate - strain segment IDs — e.g. `Af293_resequence2:Chr1_A_fumigatus_Af293:1-100:f`. This is not a - documentation nit; it is a live bug in Task 1's deliverable - (`ApiCommonWebsite/.../report/bed/util/StrainSegmentId.java:22`, plus whatever - `StrainSegmentIdTest` asserts about it) and blocks Task 6/7. +Every field is `[^:]+`, not DynSpan's greedy `(.*)`, so an ID carrying an extra colon is +**rejected** rather than mis-parsed into a different segment. `':'` is the only excluded +character because it is the only delimiter. + +**Defect found and fixed in review — the strain group was originally `[^:_]+`.** The `_` +exclusion was added on the belief that no strain name contains an underscore ("0 of +6,119"), as inert insurance against an ambiguous `_` FASTA key. That belief +was **false**: 1,494 of the 6,119 strain names contain an underscore (§2, re-measured +2026-07-31), so `parse()` was silently rejecting roughly a quarter of all legitimate strain +segment IDs — e.g. `Af293_resequence2:Chr1_A_fumigatus_Af293:1-100:f`. Fixed in +`65126443a`, with regression tests over the real underscore-bearing names; those five tests +fail against the old pattern and pass against the new one. + +The lesson is worth keeping, because the exclusion could not have worked anyway: narrowing +the grammar cannot make `_` reversible, since reference sequence IDs contain +underscores too (`Chr1_A_fumigatus_Af293`), so `A_B`+`C` is indistinguishable from +`A`+`B_C`. The real defense is never reversing the key — see §3.1. The ambiguity the exclusion was meant to prevent is real (strain `A_B` + sequence `C` and strain `A` + sequence `B_C` both yield the key `A_B_C`) but cannot be prevented by From 47f9e14364a30af1825f9ff9a0410b0fc6622aae Mon Sep 17 00:00:00 2001 From: John Brestelli Date: Fri, 31 Jul 2026 16:38:24 -0400 Subject: [PATCH 20/42] Apply Task 5 review findings to the strain segment record and question Turn off the basket, drop a costly sort, and stop the comments claiming guarantees WDK does not make. - useBasket="false" on StrainSegmentRecordClass. The default is true (RecordClass.java:303), so WdkModel.addBasketReferences injected StrainSegmentRecordClasses_StrainSegmentRecordClass{Realtime,Snapshot}Basket questions and user_baskets id queries, and RecordClassFormatter:75 advertised basket affordances for a class with no record page and no saved strategies. Matches fileRecord.xml, userfileRecords.xml, ajaxRecords.xml; verified gone from wdkXml -model FungiDB. - Drop sorting="strain_seq_id asc". AnswerValue.prepareSortingSqls splices a QueryColumnAttributeField's attribute query into the ordering SQL, so it re-ran the apidb.indel prefix-sum aggregation just to order rows. Undeclared, WDK sorts on the idAttribute against the id query alone; strain_seq_id is a pure function of the PK, so nothing is lost. - Correct a false precedent: DynSpansByLocation is only an sqlQuery with no question and no references anywhere, so it says nothing about service-API reachability. Cite DynSpansBySegIds alone, and record in spec 5.1 that the param shape is modeled on a query no question has ever exercised. - Rewrite the record-class comment as intent, not a guarantee: WDK always injects a _default summary view, a _default "Overview" record view, and DefaultJsonReporter, and the nine columnAttributes are display attributes (they must stay so; the BED reporter reads them). - Spec: add the model-side element as a third Task 6 deliverable and track it in the risk table, since a missing reporter means no BED download and no error anywhere. - attributesList summary now mirrors what the BED download consumes; add a alongside ; note that the querySet StrainSegmentId is unrelated to the Java class of the same name. Co-Authored-By: Claude Opus 5 --- .../questions/strainSegmentQuestions.xml | 44 ++++++++- .../wdk/model/records/strainSegmentRecord.xml | 45 ++++++++- .../plans/2026-07-31-strain-segment-record.md | 55 ++++++++--- ...2026-07-31-strain-segment-record-design.md | 93 +++++++++++++++---- 4 files changed, 198 insertions(+), 39 deletions(-) diff --git a/Model/lib/wdk/model/questions/strainSegmentQuestions.xml b/Model/lib/wdk/model/questions/strainSegmentQuestions.xml index 187e297c2..599071283 100644 --- a/Model/lib/wdk/model/questions/strainSegmentQuestions.xml +++ b/Model/lib/wdk/model/questions/strainSegmentQuestions.xml @@ -3,24 +3,58 @@ + `wb ontology` is not required. + + The queryRef below names the querySet 'StrainSegmentId', which follows the repo + convention for id query sets (SpanId, GeneId, FileId). It is unrelated to the + Java class org.apidb.apicommon.model.StrainSegmentId. --> - + + Given a genomic location in reference coordinates and a strain, return the equivalent segment in that strain's consensus sequence coordinates. + + bed reporter for BED/FASTA download; it has no record page + and no category-tree placement. + ]]> + + diff --git a/Model/lib/wdk/model/records/strainSegmentRecord.xml b/Model/lib/wdk/model/records/strainSegmentRecord.xml index 168a28abc..7cc4664f1 100644 --- a/Model/lib/wdk/model/records/strainSegmentRecord.xml +++ b/Model/lib/wdk/model/records/strainSegmentRecord.xml @@ -4,10 +4,35 @@ + change, adds this exact element here: + + + + scopes is "results" only. The precedents dynSpanRecord.xml:35 and + genomicRecords.xml:121 use "results,record", but this class must not claim a + 'record' scope: there is no record page to download from. --> diff --git a/docs/superpowers/plans/2026-07-31-strain-segment-record.md b/docs/superpowers/plans/2026-07-31-strain-segment-record.md index fc26a9042..266959e35 100644 --- a/docs/superpowers/plans/2026-07-31-strain-segment-record.md +++ b/docs/superpowers/plans/2026-07-31-strain-segment-record.md @@ -770,13 +770,22 @@ bash ~/workspaces/agentic-veupath-dev/bin/veup-git-sync.sh fungidb This is the first task that can break the build, because it is the task that makes the model load the new files. -The record class is deliberately bare: no tables, no summary view, no display attributes, -and only the `bed` reporter. A record-page request should fail rather than render an -empty page. `doNotTest="true"` keeps it out of the WDK sanity tests, matching DynSpan. +The record class is deliberately bare: no tables and no reporters beyond `bed` (added in +Task 6). `useBasket="false"` is required — the default is `true` +(`RecordClass.java:303`) and would inject basket questions and client affordances (spec +§6). `doNotTest="true"` keeps it out of the WDK sanity tests, matching DynSpan. + +"No record page" is an **intent, not a guarantee**: WDK injects a `_default` summary view, +a `_default` "Overview" record view, and `DefaultJsonReporter` unconditionally (spec §6), +and the nine `columnAttribute`s *are* display attributes — they must stay that way because +the BED reporter reads them. A record-page request is not a supported operation and is not +verified to do anything graceful. **No `individuals.txt` entry.** Omitting the row is what keeps the search out of the category tree while leaving it addressable by name through the service API — the same -mechanism `DynSpansBySegIds` and `DynSpansByLocation` already rely on (spec §6.1). +mechanism `DynSpansBySegIds` relies on (spec §6.1). Note `DynSpansByLocation` is **not** a +second precedent: it is only an `` (`spanQueries.xml:266`) with no `` +and no references anywhere, i.e. a dead id query (spec §5.1). **Files:** - Create: `ApiCommonModel/Model/lib/wdk/model/records/strainSegmentRecord.xml` @@ -792,13 +801,20 @@ mechanism `DynSpansBySegIds` and `DynSpansByLocation` already rely on (spec §6. @@ -810,10 +826,10 @@ mechanism `DynSpansBySegIds` and `DynSpansByLocation` already rely on (spec §6. - + @@ -842,21 +858,36 @@ mechanism `DynSpansBySegIds` and `DynSpansByLocation` already rely on (spec §6. + appear in the category tree. Still reachable by name via the service API, the + same mechanism DynSpansBySegIds relies on (spanQuestions.xml:15). + DynSpansByLocation is NOT a second precedent: it is a bare sqlQuery with no + question and no references (spec section 5.1). --> - + + Given a genomic location in reference coordinates and a strain, return the equivalent segment in that strain's consensus sequence coordinates. + + + + diff --git a/docs/superpowers/specs/2026-07-31-strain-segment-record-design.md b/docs/superpowers/specs/2026-07-31-strain-segment-record-design.md index f632d61d4..08cc37a72 100644 --- a/docs/superpowers/specs/2026-07-31-strain-segment-record-design.md +++ b/docs/superpowers/specs/2026-07-31-strain-segment-record-design.md @@ -226,6 +226,14 @@ Input is a **single reference location**, modeled on `DynSpansByLocation` (`spanQueries.xml:266`), not a `datasetParam` — so `SpanParams.span_id` and its `recordClassRef` to DynSpan are not involved. +**Caveat on that precedent, verified 2026-07-31:** `DynSpansByLocation` is *only* an +``. No `` wraps it, and nothing references it anywhere in +ApiCommonModel, ApiCommonWebsite, ApiCommonWebService, EbrcModelCommon or +EbrcWebsiteCommon — it is a dead id query. So "modeled on `DynSpansByLocation`" means +modeled on a query **no question has ever exercised**: the param shape below has no live +precedent and inherits no operational confidence. Worth knowing when the params +misbehave — there is no working sibling to diff against. + Params: `strain`, `sequenceId`, `start`, `end`, `strand`. **There is no organism param.** ### 5.1.1 Why no organism input @@ -436,13 +444,33 @@ range maps to nothing. So: ## 6. Record class (`ApiCommonModel`) `StrainSegmentRecordClasses.StrainSegmentRecordClass`, -`urlName="strain-genomic-segment"`, `doNotTest="true"`. - -Reporters: **`bed` only** (see §8). Attributes: a `columnAttribute` for each of the nine -non-primary-key columns §5.3 emits — the passed-through `strain`, `ref_seq`, `ref_start`, -`ref_end` included, since §7's defline consumes them — plus the `idAttribute` over the -`source_id`/`project_id` primary key. No tables, no summary view, no text attributes for -display. +`urlName="strain-genomic-segment"`, `doNotTest="true"`, **`useBasket="false"`**. + +`useBasket="false"` is required, not cosmetic: `RecordClass.useBasket` defaults to `true` +(`RecordClass.java:303`), and `WdkModel.addBasketReferences` (`WdkModel.java:645-651`) +then injects `..._RealtimeBasket` / `..._SnapshotBasket` questions and their `user_baskets` +id queries, while `RecordClassFormatter.java:75` reports `useBasket` to the client so the +results table offers basket affordances. Matches every other internal record class here +(`fileRecord.xml:4`, `userfileRecords.xml:4`, `ajaxRecords.xml:16,35`); DynSpan leaving the +default alone is not a counterexample, since DynSpan is user-facing. + +Reporters: **`bed` only** (plus WDK's unavoidable default JSON reporter — see below). +Attributes: a `columnAttribute` for each of the nine non-primary-key columns §5.3 emits — +the passed-through `strain`, `ref_seq`, `ref_start`, `ref_end` included, since §7's defline +consumes them — plus the `idAttribute` over the `source_id`/`project_id` primary key. No +tables. + +**What "no record page" does and does not mean.** It is an intent, not something WDK +enforces. WDK unconditionally injects a `_default` summary view +(`RecordClass.java:1306` → `SummaryView.createSupportedSummaryViews`), a `_default` record +view displayed as "Overview" (`RecordClass.java:1346` → +`RecordView.createSupportedRecordViews`), and `DefaultJsonReporter` +(`RecordClass.java:1137-1139`). The nine `columnAttribute`s are ordinary *display* +attributes — they carry `displayName` and are not `internal="true"`, and they must stay +that way because the BED reporter reads them. The real basis for "non-user-facing" is +narrower and sufficient: no tables, no reporters beyond `bed`, no ontology rows, so nothing +wires up a record page. A record-page request is not a supported operation and is not +verified to degrade gracefully. Register the new files in `Model/lib/wdk/apiCommonModel.xml`, alongside the existing span imports at lines 419-423. @@ -473,11 +501,15 @@ Scope combinations across the 202 `targetType=search` rows, measured 2026-07-31: **Decision: add no row at all for the new search.** Two mechanisms could hide it, and omission is the right one here: -- **Omission** — `DynSpansBySegIds` (commented "SegIds only WEBSERVICES", - `spanQueries.xml:7`) and `DynSpansByLocation` are both absent from `individuals.txt` - and the model builds. A question stays addressable by name through the service API - regardless of ontology presence. This is the precedent for a *hand-written* - webservice-only span search — exactly our case. +- **Omission** — `DynSpansBySegIds` (`spanQuestions.xml:15`, commented "SegIds only + WEBSERVICES" at `spanQueries.xml:7`) is absent from `individuals.txt` and the model + builds. A question stays addressable by name through the service API regardless of + ontology presence. This is *the* precedent for a hand-written webservice-only span + search — exactly our case, and it is the **only** one. An earlier draft of this spec + also cited `DynSpansByLocation`; that was wrong and is corrected in §5.1 — + `DynSpansByLocation` is only an `` (`spanQueries.xml:266`) with no + `` and no references anywhere, so it demonstrates nothing about service-API + reachability. - **`internal` + `webservice`** — all 42 users are *injected per-dataset* searches, which need ontology presence for categorization to work. Not our situation. @@ -486,12 +518,37 @@ Consequence for verification: the search must be **absent** from does **not** touch categorization — so `wb model` suffices and `wb ontology` is not required. (It would be required if a row were ever added.) -## 7. BED reporter (`ApiCommonWebsite`) +## 7. BED reporter (`ApiCommonWebsite` **and one model-side element**) + +Three deliverables, not two. The third is easy to lose because it lives in a different +repo from the other two: + +1. `BedStrainSegmentReporter extends BedReporter` (`ApiCommonWebsite`) +2. `StrainSegmentFeatureProvider implements BedFeatureProvider` (`ApiCommonWebsite`) +3. the `` element on the record class (`ApiCommonModel`, + `Model/lib/wdk/model/records/strainSegmentRecord.xml`) — **exact shape**: + +```xml + +``` + +`scopes="results"`, *not* `"results,record"`. The precedents `dynSpanRecord.xml:35` and +`genomicRecords.xml:121` use `"results,record"`, but this class must not claim a `record` +scope: §6 gives it no record page to download from. + +`name="bed"` is the literal string §9's verification passes as `reportName`, so it must +match exactly. + +The element cannot land before the Java class: `ReporterRef.resolveReferences` does +`Class.forName` on the implementation at model-load time and hard-fails the build if it is +absent. Hence it ships in Task 6 alongside the reporter, not in Task 5 — which is exactly +why it is at risk of being forgotten (see §10): **omit it and there is no BED download and +no error anywhere**, because nothing in the model or the Java code references the reporter +by name at build time. -`BedStrainSegmentReporter extends BedReporter` + `StrainSegmentFeatureProvider -implements BedFeatureProvider`, following `BedGenomicSequenceReporter` / -`GenomicSequenceFeatureProvider` — the existing precedent for a provider that computes -coordinates from **attributes** rather than from the PK. +`BedStrainSegmentReporter` / `StrainSegmentFeatureProvider` follow +`BedGenomicSequenceReporter` / `GenomicSequenceFeatureProvider` — the existing precedent +for a provider that computes coordinates from **attributes** rather than from the PK. `getRequiredAttributeNames()` = `{strain_seq_id, strain_start, strain_end, organism}`. Strain is parsed from the PK, so it needs no attribute. @@ -566,3 +623,5 @@ dense options: *Cryptococcus neoformans* H99 (875), *Candida auris* B8441 (502), | `(name, na_sequence_id)` uniqueness | true today, unconstrained; §5.3 grouping converts a violation to an error | | Prefix-sum cost | heaviest strain/sequence pair carries ~115k events; the `location <= ref_end` bound plus `ix0`/`ix1` should hold, but measure on Af293 | | Categorization rebuild | not applicable — §6.1 adds no ontology node, so `wb model` suffices. If a row is ever added, it becomes `wb ontology` (a superset of `wb model`). | +| **Model-side `` element silently missing** | **open until Task 6** — it ships in a different repo from the two Java classes (§7.3), and nothing references it at build time. Omit it and there is no BED download and **no error anywhere**: `/service/record-types/strain-genomic-segment` simply lists no `bed` format. Verification: that endpoint's `formats` must contain `bed` with `scopes` = `results` only. | +| Reporter `scopes` over-claimed | **open until Task 6** — copying `scopes="results,record"` from `dynSpanRecord.xml:35` / `genomicRecords.xml:121` would advertise a record-scope download on a class with no record page. Must be `scopes="results"`. | From 06c656fded9fd6bbf20e26c7ddbda904763bb53b Mon Sep 17 00:00:00 2001 From: John Brestelli Date: Fri, 31 Jul 2026 16:48:15 -0400 Subject: [PATCH 21/42] Correct comment claims found in the Task 5 re-review The rewrite that closed the previous round introduced a new false claim of exactly the kind that round existed to remove, plus two overclaims: - The naming-caution comment, whose only job is to stop a reader conflating the querySet StrainSegmentId with the Java class of the same name, gave the class's package as org.apidb.apicommon.model.StrainSegmentId. Nothing exists there; it is org.apidb.apicommon.model.report.bed.util.StrainSegmentId. A disambiguating comment that resolves to nothing is worse than none. - "no reference anywhere in ApiCommonModel..." for DynSpansByLocation was absolute and false: the Model/vp2TuningTablesEffort inventories name it twice. The substantive point survives and is stronger for being precise, because tableUsageMap.json records an EMPTY usage list for it -- an independent inventory agreeing nothing uses it. - The attributesList comment claimed the summary "mirrors exactly what the BED download consumes". It does not: per spec section 7 the provider needs four attributes and reads strain and ref_seq out of the primary key instead, so the claim was false by two columns against the section it cited. It also cited getRequiredAttributeNames() in the present indicative for a class Task 6 has not written yet. Also: two line cites had drifted (addBasketReferences is at 646-652, prepareSortingSqls spans 748-824); the sorting fallback now traces from Question.getSortingAttributeMap, where the removed attribute actually lived, rather than starting at the record class; and spec 5.1.1 attributed source_id_uniq to dots.ExternalNaSequence, which is a view and can hold no index. The index is on the base table nasequenceimp, making the uniqueness guarantee broader than the spec claimed. Verified: wb model builds and the webapp reloads. --- .../questions/strainSegmentQuestions.xml | 33 ++++++++++++------- .../wdk/model/records/strainSegmentRecord.xml | 4 +-- ...2026-07-31-strain-segment-record-design.md | 18 +++++++--- 3 files changed, 36 insertions(+), 19 deletions(-) diff --git a/Model/lib/wdk/model/questions/strainSegmentQuestions.xml b/Model/lib/wdk/model/questions/strainSegmentQuestions.xml index 599071283..50fd9856e 100644 --- a/Model/lib/wdk/model/questions/strainSegmentQuestions.xml +++ b/Model/lib/wdk/model/questions/strainSegmentQuestions.xml @@ -7,34 +7,43 @@ same mechanism DynSpansBySegIds relies on (spanQuestions.xml:15, no individuals.txt row). DynSpansByLocation is NOT a second precedent, despite looking like one: it exists only as an sqlQuery (queries/spanQueries.xml:266) - with no question wrapping it and no reference anywhere in ApiCommonModel, - ApiCommonWebsite, ApiCommonWebService, EbrcModelCommon or EbrcWebsiteCommon. It - is a dead id query, so it says nothing about service-API reachability. Because + with no question wrapping it and nothing consuming it across ApiCommonModel, + ApiCommonWebsite, ApiCommonWebService, EbrcModelCommon or EbrcWebsiteCommon. The + only other mentions are the Model/vp2TuningTablesEffort inventories, where + tableUsageMap.json records an EMPTY usage list for it, independently agreeing it + has no users. It is a dead id query, so it says nothing about service-API + reachability. Because nothing about categorization changes, `wb model` is the correct build target; `wb ontology` is not required. The queryRef below names the querySet 'StrainSegmentId', which follows the repo convention for id query sets (SpanId, GeneId, FileId). It is unrelated to the - Java class org.apidb.apicommon.model.StrainSegmentId. --> + Java class org.apidb.apicommon.model.report.bed.util.StrainSegmentId. --> - diff --git a/Model/lib/wdk/model/records/strainSegmentRecord.xml b/Model/lib/wdk/model/records/strainSegmentRecord.xml index 7cc4664f1..bb7c63a0a 100644 --- a/Model/lib/wdk/model/records/strainSegmentRecord.xml +++ b/Model/lib/wdk/model/records/strainSegmentRecord.xml @@ -19,7 +19,7 @@ useBasket="false" is load bearing, not decoration. RecordClass.useBasket defaults to true (RecordClass.java:303); WdkModel.addBasketReferences - (WdkModel.java:645..651) then injects synthetic '..._RealtimeBasket' and + (WdkModel.java:646..652) then injects synthetic '..._RealtimeBasket' and '..._SnapshotBasket' questions plus id queries over user_baskets, and RecordClassFormatter (line 75) reports useBasket to the client so the results table offers basket affordances. None of that makes sense for a class with no @@ -31,7 +31,7 @@ Naming caution for later tasks: the querySet 'StrainSegmentId' that the question references follows the repo convention for id query sets (SpanId, GeneId, FileId) and is unrelated to the Java class - org.apidb.apicommon.model.StrainSegmentId that parses the primary key. Same + org.apidb.apicommon.model.report.bed.util.StrainSegmentId that parses the primary key. Same string, different things. Works on every project: no includeProjects/excludeProjects anywhere, including diff --git a/docs/superpowers/specs/2026-07-31-strain-segment-record-design.md b/docs/superpowers/specs/2026-07-31-strain-segment-record-design.md index 08cc37a72..4e6e1c4d6 100644 --- a/docs/superpowers/specs/2026-07-31-strain-segment-record-design.md +++ b/docs/superpowers/specs/2026-07-31-strain-segment-record-design.md @@ -227,9 +227,12 @@ Input is a **single reference location**, modeled on `DynSpansByLocation` `recordClassRef` to DynSpan are not involved. **Caveat on that precedent, verified 2026-07-31:** `DynSpansByLocation` is *only* an -``. No `` wraps it, and nothing references it anywhere in +``. No `` wraps it, and nothing *consumes* it across ApiCommonModel, ApiCommonWebsite, ApiCommonWebService, EbrcModelCommon or -EbrcWebsiteCommon — it is a dead id query. So "modeled on `DynSpansByLocation`" means +EbrcWebsiteCommon — it is a dead id query. (It is not literally unmentioned: the +`Model/vp2TuningTablesEffort` inventories name it twice, and `tableUsageMap.json` records +`"SpanId.DynSpansByLocation": []` — an empty usage list, which independently corroborates +that nothing uses it.) So "modeled on `DynSpansByLocation`" means modeled on a query **no question has ever exercised**: the param shape below has no live precedent and inherits no operational confidence. Worth knowing when the params misbehave — there is no working sibling to diff against. @@ -246,8 +249,13 @@ organism as well only creates a consistency question the query then has to answe Dropping it removes three problems at once: - **No partition-key concern.** Resolve the sequence through unpartitioned - `dots.ExternalNaSequence` (`source_id_uniq` index; carries `na_sequence_id`, `taxon_id`, - `length`) rather than `webready.GenomicSeqAttributes_p`. Same table path §5.3 already + `dots.ExternalNaSequence` (carries `na_sequence_id`, `taxon_id`, `length`; `source_id` + uniqueness is enforced by `source_id_uniq` on the **base table** `dots.nasequenceimp` — + `ExternalNaSequence` is a view, so it holds no index of its own. The guarantee is + therefore stronger than "unique among external sequences": it is unique across all of + `nasequenceimp`, which is what makes "one input location yields one record" structural + rather than a data accident. Measured: 10,331,542 rows, 0 duplicate non-null `source_id`, + the only gap being 17 NULLs that can never match `= $$sequenceId$$`.) rather than `webready.GenomicSeqAttributes_p`. Same table path §5.3 already uses, so the two queries become consistent instead of divergent. - **No `@PROJECT_ID@`.** `project_id` for the PK comes from `apidb.organism.project_name` via `taxon_id` — unpartitioned, correct on every project, still one query. @@ -447,7 +455,7 @@ range maps to nothing. So: `urlName="strain-genomic-segment"`, `doNotTest="true"`, **`useBasket="false"`**. `useBasket="false"` is required, not cosmetic: `RecordClass.useBasket` defaults to `true` -(`RecordClass.java:303`), and `WdkModel.addBasketReferences` (`WdkModel.java:645-651`) +(`RecordClass.java:303`), and `WdkModel.addBasketReferences` (`WdkModel.java:646-652`) then injects `..._RealtimeBasket` / `..._SnapshotBasket` questions and their `user_baskets` id queries, while `RecordClassFormatter.java:75` reports `useBasket` to the client so the results table offers basket affordances. Matches every other internal record class here From db185b8f33df7e7cd899ba811684888d3279ef8b Mon Sep 17 00:00:00 2001 From: John Brestelli Date: Fri, 31 Jul 2026 16:58:52 -0400 Subject: [PATCH 22/42] Register the BED reporter on the strain segment record class scopes="results" only, not "results,record" as dynSpanRecord.xml and genomicRecords.xml use: this class has no record page to download from. name="bed" is the literal reportName a download request passes, so it must match exactly, and nothing references the reporter by name at build time, which is why omitting the element yields no BED download and no error anywhere. Verified with wb model: ReporterRef.resolveReferences Class.forName's the implementation at model-load time, so a successful load is the proof that BedStrainSegmentReporter is compiled into the deployed webapp. Co-Authored-By: Claude Opus 5 --- .../wdk/model/records/strainSegmentRecord.xml | 29 ++++++++++--------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/Model/lib/wdk/model/records/strainSegmentRecord.xml b/Model/lib/wdk/model/records/strainSegmentRecord.xml index bb7c63a0a..cdd36e4e3 100644 --- a/Model/lib/wdk/model/records/strainSegmentRecord.xml +++ b/Model/lib/wdk/model/records/strainSegmentRecord.xml @@ -54,22 +54,23 @@ - + The element could not land in Task 5: ReporterRef.resolveReferences does a + Class.forName on the implementation at model-load time and hard-fails the build + when the class is absent. So a successful model load is itself the proof that + BedStrainSegmentReporter compiled into the deployed webapp. --> + From 5431e0265ac171e5868e145907e06aec7f158606 Mon Sep 17 00:00:00 2001 From: John Brestelli Date: Fri, 31 Jul 2026 17:01:28 -0400 Subject: [PATCH 23/42] Record that wb model does not compile ApiCommonWebsite/Model Task 6's first wb model failed with 'Implementation class for reporter bed ... cannot be found'. The XML registration was correct; the class had never been deployed. Verified in gus_home/bin/wb: the model target runs bld EbrcModelCommon/Model and bld ApiCommonModel/Model only, site covers Website/Site, ontology adds Presenters/Model, and no wb target builds ApiCommonWebsite/Model at all. mvn test compiles to target/classes, which is not on wdkXml's classpath. Worth documenting because the error names the wrong file: it reads as a bad reporter element and invites reverting a correct registration. The fix is bld ApiCommonWebsite/Model first. --- .../plans/2026-07-31-strain-segment-record.md | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/docs/superpowers/plans/2026-07-31-strain-segment-record.md b/docs/superpowers/plans/2026-07-31-strain-segment-record.md index 266959e35..e598ced7f 100644 --- a/docs/superpowers/plans/2026-07-31-strain-segment-record.md +++ b/docs/superpowers/plans/2026-07-31-strain-segment-record.md @@ -42,6 +42,22 @@ bash ~/workspaces/agentic-veupath-dev/bin/veup-logs.sh fungidb since t1 `wb ontology` is **not** needed: this work adds no ontology node (spec §6.1). `wb model` suffices. +> **`wb model` does NOT compile `ApiCommonWebsite/Model`.** Verified in `gus_home/bin/wb`: the +> `model` target runs `bld EbrcModelCommon/Model; bld ApiCommonModel/Model` and nothing else; +> `site` covers `Website/Site` and `ontology` adds `Presenters/Model`. No `wb` target builds +> `ApiCommonWebsite/Model`. So after editing Java there (Tasks 1 and 6), run: +> +> ```bash +> ssh cedar 'bash -lc "source /var/www/jbrestel.fungidb.org/etc/setenv && bld ApiCommonWebsite/Model"' +> ``` +> +> **The failure mode is misleading, which is why this is worth knowing:** `mvn test` compiles +> to `target/classes`, which is not on `wdkXml`'s classpath — the jar must be installed into +> `gus_home/lib/java`. Skip the `bld` and `wb model` fails with `Implementation class for +> reporter 'bed' … cannot be found` / `ClassNotFoundException`, which reads as "the XML +> registration is wrong" when in fact the XML is fine and the class simply was never deployed. +> Do not respond to that error by reverting the `` element. + --- ## File structure From 6c0bfd8d3feb80a3355a4602c84475cd06233f6f Mon Sep 17 00:00:00 2001 From: John Brestelli Date: Fri, 31 Jul 2026 17:04:16 -0400 Subject: [PATCH 24/42] Correct the illustrative defline to what DeflineBuilder actually emits Spec section 7 showed the two coordinate ranges as 'ref 100 to 200 | strain 143 to 241'. Nothing emits that. DeflineBuilder.appendPosition renders ', forward strand, 100 to 200', so matching the example would have meant adding a builder method or hand-rolling strings, and the invented form silently dropped the strand. The name column is declared Free and the normative requirement is only that it carry provenance through DeflineBuilder honouring RequestedDeflineFields, which the implementation does. Corrected so nobody 'fixes' working code to match a retired example, and so Task 7 compares against the real string. --- .../2026-07-31-strain-segment-record-design.md | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/docs/superpowers/specs/2026-07-31-strain-segment-record-design.md b/docs/superpowers/specs/2026-07-31-strain-segment-record-design.md index 4e6e1c4d6..34e65fe8c 100644 --- a/docs/superpowers/specs/2026-07-31-strain-segment-record-design.md +++ b/docs/superpowers/specs/2026-07-31-strain-segment-record-design.md @@ -572,12 +572,24 @@ So `chrom = strain_seq_id`, and the `name` column carries provenance via `DeflineBuilder`, honouring `RequestedDeflineFields` as `DynSpanFeatureProvider` does (`DynSpanFeatureProvider.java:57-72`): +The shape `DeflineBuilder.appendPosition` actually produces, with all fields requested +(`deflineType=full`): + ``` ->A0003:AACB03000001:100-200:f | Aspergillus fumigatus Af293 | A0003 | ref 100 to 200 | strain 143 to 241 | segment_length=99 +>A0003:AACB03000001:100-200:f | Aspergillus fumigatus Af293 | A0003 | AACB03000001, forward strand, 100 to 200 | A0003_AACB03000001, forward strand, 143 to 241 | segment_length=99 ``` Carrying both coordinate systems is the payoff for choosing reference coordinates in -the PK (§3). +the PK (§3). The two ranges stay distinguishable because the first names the reference +sequence and the second carries the strain prefix. + +**An earlier revision illustrated the two ranges as `ref 100 to 200 | strain 143 to 241`.** +Nothing ever emitted that. Matching it would have meant adding a `DeflineBuilder` method or +hand-rolling strings — duplicating the builder for cosmetics — and it silently dropped the +strand, which `appendPosition` includes. The `name` column is declared **Free** above and +the normative requirement is only that it carry provenance through `DeflineBuilder` honouring +`RequestedDeflineFields`, which the implementation satisfies literally. Recorded so nobody +"fixes" working code to match a retired example, and so §9 verifies against the real string. ## 8. Deferred: seqret wiring From 2c60a747fbf4e7ae8dd1693b1094f8b004ca35d9 Mon Sep 17 00:00:00 2001 From: John Brestelli Date: Fri, 31 Jul 2026 17:22:19 -0400 Subject: [PATCH 25/42] Update the three comments Task 6 falsified Task 6's re-review found the inverse of the usual defect: comments that were accurate when written and that landing Task 6 made false. Each was scoped "as of this commit", which is defensible authorship but reads as wrong to anyone looking at the current tree. The consequential one is in strainSegmentAttributeQueries.xml, sitting directly beside the deliberately unclamped strain_start/strain_end and telling the next reader that nothing downstream rejects an inverted segment. Something does, since a3c062e03. Leaving those columns unclamped is safe ONLY because that guard exists, so the comment now says that explicitly and names all three conditions the guard checks. Spec 5.3.2 likewise flipped from "Not yet implemented" to implemented, and now enumerates the full contract: the deletion inversion, strain_start < 1, and the strain_seq_id-versus-primary-key agreement check added during hardening. It also states the converse, which is the part worth keeping: if that guard is ever removed, these columns must be clamped instead. Also hardened the plan's test invocations. This module inherits surefire 2.12.4, where -Dtest=A+B matches ZERO tests; combined with the plan's -DfailIfNoTests=false that reported BUILD SUCCESS having run nothing. Switched to -DfailIfNoTests=true, corrected the stale "Tests run: 12" expectation to the 20 + 10 = 30 actually observed from a comma-separated run, and recorded the trap where someone will hit it. Verified: wb model builds and the webapp reloads. --- .../questions/strainSegmentQuestions.xml | 4 ++-- .../records/strainSegmentAttributeQueries.xml | 9 ++++++--- .../plans/2026-07-31-strain-segment-record.md | 20 +++++++++++++++---- ...2026-07-31-strain-segment-record-design.md | 18 +++++++++++++---- 4 files changed, 38 insertions(+), 13 deletions(-) diff --git a/Model/lib/wdk/model/questions/strainSegmentQuestions.xml b/Model/lib/wdk/model/questions/strainSegmentQuestions.xml index 50fd9856e..1fbeaeca9 100644 --- a/Model/lib/wdk/model/questions/strainSegmentQuestions.xml +++ b/Model/lib/wdk/model/questions/strainSegmentQuestions.xml @@ -27,8 +27,8 @@ @@ -69,12 +69,12 @@ This search allows you to identify organisms based on the name of the organism. recordClassRef="OrganismRecordClasses.OrganismRecordClass"> diff --git a/Model/lib/wdk/model/records/organismAttributeQueries.xml b/Model/lib/wdk/model/records/organismAttributeQueries.xml index 31da7de60..248411ee5 100644 --- a/Model/lib/wdk/model/records/organismAttributeQueries.xml +++ b/Model/lib/wdk/model/records/organismAttributeQueries.xml @@ -101,7 +101,7 @@ - + @@ -158,7 +158,7 @@ nullif(oa.gocount,0) as gocount, nullif(oa.tfbscount,0) as tfbscount, nullif(oa.proteomicscount,0) as proteomicscount, - nullif(oa.communitycount,0) as communitycount, + --nullif(oa.communitycount,0) as communitycount, CASE oa.isOrganellar WHEN 0 THEN 'no' ELSE 'yes' END as isOrganellar_flag, CASE oa.arraygenecount WHEN 0 THEN 'none' ELSE 'yes' END as array_flag, CASE oa.chipchipgenecount WHEN 0 THEN 'none' ELSE 'yes' END as chipchip_flag, @@ -168,7 +168,9 @@ CASE oa.ecnumbercount WHEN 0 THEN 'none' ELSE 'yes' END as ec_flag, CASE oa.gocount WHEN 0 THEN 'none' ELSE 'yes' END as go_flag, CASE oa.rtpcrcount WHEN 0 THEN 'none' ELSE 'yes' END as rtpcr_flag, - CASE oa.communitycount WHEN 0 THEN 'none' ELSE 'yes' END as community_flag, + -- was: CASE oa.communitycount WHEN 0 THEN 'none' ELSE 'yes' END; communityCount is + -- disabled in apiTuningManager.xml, and 'none' is what it yielded for a zero count + 'none' as community_flag, CASE oa.tfbscount WHEN 0 THEN 'none' ELSE 'yes' END as tfbs_flag, CASE oa.hasHTSIsolate WHEN 0 THEN 'none' ELSE 'yes' END as hasHTSIsolate_flag, --CASE oa.hasPopset WHEN 0 THEN 'none' ELSE 'yes' END as hasPopset_flag, diff --git a/Model/lib/wdk/model/records/organismRecords.xml b/Model/lib/wdk/model/records/organismRecords.xml index f2ffe4de8..4fa574883 100644 --- a/Model/lib/wdk/model/records/organismRecords.xml +++ b/Model/lib/wdk/model/records/organismRecords.xml @@ -180,8 +180,10 @@ help="Number of genes that have evidence of protein expression based on Mass Spectrometry studies" align="right"/> - + From 0d24155ca2d75ddd5b0c973f53c1cf250eacfa28 Mon Sep 17 00:00:00 2001 From: John Brestelli Date: Wed, 29 Jul 2026 23:43:17 -0400 Subject: [PATCH 30/42] Inject the DeRisi time series questions instead of hardcoding them These two questions were written by hand in geneQuestions.xml while the attribute their summaries name is generated by the presenter: summary="...,exprGraphAttrpfal3D7_microarrayExpression_Derisi_TimeSeries_RSRC_expr_graph" Expression.java:136 builds that name and injects the textAttribute that defines it. So a hand-written question depended on generated output, coupled only by a name string with nothing enforcing that both exist. On an instance where this dataset is not loaded, the presenter is skipped and the question survives, referencing an attribute nobody generated: Summary attribute field [exprGraphAttrpfal3D7_..._expr_graph] defined in question [GeneQuestions.GenesByMicroarraypfal3D7_..._RSRC] is invalid The questions are curated rather than generic -- they use the PFTimeSeries vocabulary queries, an extra samples_fc_comp_generic param, and fold_change_chosen_display in place of fold_change_chosen -- which is why the presenter sets hasMultipleSamples, hasPercentileData and hasPageData to false and why they were hardcoded in the first place. So they move to their own microarrayDeRisiTimeSeries.dst rather than being folded into the generic expression templates, and MicroarrayDeRisiTimeSeries injects them. Question and attribute now appear and disappear together. Only three values are parameterised -- ${datasetName} for the question and graph attribute names, ${includeProjects} and ${includeProjectsExcludeEuPathDB} for scoping, both already set by Expression.java to precisely the literals that were hardcoded. Display strings stay literal because they describe this experiment. addModelReferences now derives the question names from getDatasetName() rather than spelling out the dataset, so the references cannot drift from what is injected. Not yet verified: that the generated question is equivalent to the deleted static XML. Nothing is generated on an instance where this dataset is absent, so this branch can only show that its absence is handled. Before this reaches a full-data site, run a build with the dataset declared and diff the generated question against the static version in this commit's parent. Co-Authored-By: Claude Opus 5 (cherry picked from commit 28b06e44839edea3786d693caa901fcf815be670) --- Model/lib/dst/microarrayDeRisiTimeSeries.dst | 193 ++++++++++++++++++ .../lib/wdk/model/questions/geneQuestions.xml | 181 +--------------- .../PlasmoDB/MicroarrayDeRisiTimeSeries.java | 22 +- 3 files changed, 215 insertions(+), 181 deletions(-) create mode 100644 Model/lib/dst/microarrayDeRisiTimeSeries.dst diff --git a/Model/lib/dst/microarrayDeRisiTimeSeries.dst b/Model/lib/dst/microarrayDeRisiTimeSeries.dst new file mode 100644 index 000000000..598f48f3a --- /dev/null +++ b/Model/lib/dst/microarrayDeRisiTimeSeries.dst @@ -0,0 +1,193 @@ +[templateStart] +name=microarrayDeRisiTimeSeriesFoldChangeQuestion +anchorFile=ApiCommonModel/Model/lib/wdk/model/questions/geneQuestions.xml +prop=datasetName +prop=includeProjects +prop=includeProjectsExcludeEuPathDB +>templateTextStart< + + + + + + + + + + 1 + + + + + + + + + +
+After selecting samples you have the option to take the average, minimum, or maximum expression value within each group. (If choosing only one sample from a group, the selected 'operation' will not affect your results). Time series experiments will offer an extra parameter called "Global min/max" which allows you to filter your results further. Finally, you can choose the directionality and the magnitude of the difference. For example, selecting up-regulated with a fold difference of 2 will only show results where the comparator is twice that of the reference. +

+ + + ]]> + + + + + + Display the histogram of the values of this attribute + int + + + + + Display the histogram of the values of this attribute + int + + + + + + Display the histogram of the values of this attribute + int + + + + + Display the histogram of the values of this attribute + int + + + + + Display the histogram of the values of this attribute + int + + + + + Display the histogram of the values of this attribute + int + + + + + Display the histogram of the values of this attribute + int + + + + + Display the histogram of the values of this attribute + int + + + + + Display the histogram of the values of this attribute + int + + + + + Display the histogram of the values of this attribute + int + + + + + + fold_change + + +>templateTextEnd< + +[templateStart] +name=microarrayDeRisiTimeSeriesPercentileQuestion +anchorFile=ApiCommonModel/Model/lib/wdk/model/questions/geneQuestions.xml +prop=datasetName +prop=includeProjects +prop=includeProjectsExcludeEuPathDB +>templateTextStart< + + + + + + + + + + + + + + + + +
+ + + ]]> + + + + + + Display the histogram of the values of this attribute + int + + + + + Display the histogram of the values of this attribute + int + + + + + + percentile + + +>templateTextEnd< diff --git a/Model/lib/wdk/model/questions/geneQuestions.xml b/Model/lib/wdk/model/questions/geneQuestions.xml index a2a1866e0..898f56d6f 100644 --- a/Model/lib/wdk/model/questions/geneQuestions.xml +++ b/Model/lib/wdk/model/questions/geneQuestions.xml @@ -3696,182 +3696,11 @@ For further experiment details please refer the data sources listed below. ]]> - - - - - - - - - 1 - - - - - - - - - -
-After selecting samples you have the option to take the average, minimum, or maximum expression value within each group. (If choosing only one sample from a group, the selected 'operation' will not affect your results). Time series experiments will offer an extra parameter called "Global min/max" which allows you to filter your results further. Finally, you can choose the directionality and the magnitude of the difference. For example, selecting up-regulated with a fold difference of 2 will only show results where the comparator is twice that of the reference. -

- - - ]]> - - - - - - Display the histogram of the values of this attribute - int - - - - - Display the histogram of the values of this attribute - int - - - - - - Display the histogram of the values of this attribute - int - - - - - Display the histogram of the values of this attribute - int - - - - - Display the histogram of the values of this attribute - int - - - - - Display the histogram of the values of this attribute - int - - - - - Display the histogram of the values of this attribute - int - - - - - Display the histogram of the values of this attribute - int - - - - - Display the histogram of the values of this attribute - int - - - - - Display the histogram of the values of this attribute - int - - - - - - fold_change - - - - - - - - - - - - - - - - - - - -
- - - ]]> - - - - - - Display the histogram of the values of this attribute - int - - - - - Display the histogram of the values of this attribute - int - - - - - - percentile - - + + + diff --git a/Model/src/main/java/org/apidb/apicommon/model/datasetInjector/custom/PlasmoDB/MicroarrayDeRisiTimeSeries.java b/Model/src/main/java/org/apidb/apicommon/model/datasetInjector/custom/PlasmoDB/MicroarrayDeRisiTimeSeries.java index 55370b7bb..4bf89b4b7 100644 --- a/Model/src/main/java/org/apidb/apicommon/model/datasetInjector/custom/PlasmoDB/MicroarrayDeRisiTimeSeries.java +++ b/Model/src/main/java/org/apidb/apicommon/model/datasetInjector/custom/PlasmoDB/MicroarrayDeRisiTimeSeries.java @@ -8,13 +8,22 @@ public class MicroarrayDeRisiTimeSeries extends MicroarrayTwoChannelReferenceDes public void injectTemplates() { super.injectTemplates(); - // Questions are hard coded in the model w/ the same name that would have been injected - - // we are setting hasPercentile to false so must inject these + // we are setting hasPercentile to false so must inject these setPropValue("graphTextAttrName", "pctGraphAttr" + getDatasetName() + "_pct_graph"); injectTemplate("expressionGraphAttributesPercentile"); injectTemplate("graphTextAttributeCategory"); + // These two questions are curated rather than generic — they use the PFTimeSeries + // vocabulary queries and an extra comparison-samples param — so hasMultipleSamples / + // hasPercentileData / hasPageData stay false and the questions come from + // microarrayDeRisiTimeSeries.dst instead of the generic expression templates. + // + // They used to be hardcoded in geneQuestions.xml, which meant they existed even on an + // instance where this dataset is not loaded, referencing expr/pct graph attributes that + // only exist when this presenter runs. Injecting them makes question and attribute + // appear and disappear together, so the model stays dataset driven. + injectTemplate("microarrayDeRisiTimeSeriesFoldChangeQuestion"); + injectTemplate("microarrayDeRisiTimeSeriesPercentileQuestion"); } @Override @@ -22,8 +31,11 @@ public void addModelReferences() { super.addModelReferences(); addWdkReference("TranscriptRecordClasses.TranscriptRecordClass", "question", "GeneQuestions.GenesByProfileSimilarity"); - addWdkReference("TranscriptRecordClasses.TranscriptRecordClass", "question", "GeneQuestions.GenesByMicroarraypfal3D7_microarrayExpression_Derisi_TimeSeries_RSRC"); - addWdkReference("TranscriptRecordClasses.TranscriptRecordClass", "question", "GeneQuestions.GenesByMicroarraypfal3D7_microarrayExpression_Derisi_TimeSeries_RSRCPercentile"); + // derived from the dataset rather than spelled out, so these track the injected + // question names above instead of drifting from them + String questionName = "GeneQuestions.GenesByMicroarray" + getDatasetName(); + addWdkReference("TranscriptRecordClasses.TranscriptRecordClass", "question", questionName); + addWdkReference("TranscriptRecordClasses.TranscriptRecordClass", "question", questionName + "Percentile"); } From 4656a38ab2717bcb9b9e8e806aab229917aff887 Mon Sep 17 00:00:00 2001 From: John Brestelli Date: Wed, 29 Jul 2026 23:43:36 -0400 Subject: [PATCH 31/42] Drop a dataset-specific attribute from a generic compound search CompoundQuestions.CompoundsByFoldChange named one dataset's graph column in its summary: summary="...,metaboliteProfiles_LlinasMetabolites_RSRC_metabolite_graph" That attribute is injected per metabolomics dataset by metabolomics.dst, so naming a particular dataset's copy in a generic search has two consequences: model load fails wherever that dataset is not loaded ("Summary attribute field [...] is invalid"), and where it is loaded the search shows that one dataset's graph regardless of what the user searched for. Co-Authored-By: Claude Opus 5 (cherry picked from commit e78539028ad041223af169726cbaead1dcf51374) --- Model/lib/wdk/model/questions/compoundQuestions.xml | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/Model/lib/wdk/model/questions/compoundQuestions.xml b/Model/lib/wdk/model/questions/compoundQuestions.xml index e3a8ba182..68b172b35 100644 --- a/Model/lib/wdk/model/questions/compoundQuestions.xml +++ b/Model/lib/wdk/model/questions/compoundQuestions.xml @@ -273,10 +273,15 @@ Find compounds by metabolite levels - + + />
From a27fd497c7c9dfc3074561ffa1b5ec08fe3578a8 Mon Sep 17 00:00:00 2001 From: John Brestelli Date: Fri, 31 Jul 2026 20:47:09 -0400 Subject: [PATCH 32/42] Stop the id-SQL macro from expanding inside a comment End-to-end verification failed with PSQLException "syntax error at or near WITH". Cause: two comments in the attribute query mentioned the ##WDK_ID_SQL## macro by name. WDK substitutes it by blind textual replacement, so it expanded inside the comments too, and because the injected SQL is multi-line it escaped the '--' and corrupted the statement. One of those comments was added during review to document that row counts match the injected id SQL -- a comment written to explain correctness is what broke the query. Notably wdkQuery -showQuery cannot catch this: standalone it never expands the macro at all, which this plan had already recorded as correct behaviour. Only a real answer run reaches it. Added a warning at the top of the query telling the next person never to write the macro name in a comment here. Also corrected the spec's claim that the pipeline emits one merged multi-strain FASTA. The delivered artifacts are one gzipped file per strain; deflines verified byte-identical to the chrom this record emits. --- .../model/records/strainSegmentAttributeQueries.xml | 13 ++++++++++--- .../2026-07-31-strain-segment-record-design.md | 12 +++++++++--- 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/Model/lib/wdk/model/records/strainSegmentAttributeQueries.xml b/Model/lib/wdk/model/records/strainSegmentAttributeQueries.xml index 5d290e0d8..912a379d6 100644 --- a/Model/lib/wdk/model/records/strainSegmentAttributeQueries.xml +++ b/Model/lib/wdk/model/records/strainSegmentAttributeQueries.xml @@ -27,8 +27,15 @@ _`, i.e. -**`_`**. `checkUniqueIds.sh` hard-fails (exit 125) on any duplicate -defline across the merged file, so it is **one merged multi-strain FASTA**, not one file -per strain — meaning one seqret sequence type suffices whenever §8 is picked up. +**`_`**. **Verified against real artifacts 2026-07-31** (`A17-10A-1_consensus.fa.gz`): +deflines are exactly `>A17-10A-1_Chr1_A_fumigatus_Af293`, and the `chrom` this record emits +byte-matches them. + +**Correction:** an earlier revision inferred from `checkUniqueIds.sh` that the pipeline emits +**one merged multi-strain FASTA**. The delivered artifacts are **one gzipped file per strain** +(`_consensus.fa.gz`, 9 deflines each for Af293). The uniqueness check is still +consistent with that, but the packaging inference was wrong, and §8 should not assume a single +merged file when seqret wiring is picked up. ## 3. Identity From aab2436dc2ca8c1f75b5b881b7dbbc8859b62b31 Mon Sep 17 00:00:00 2001 From: John Brestelli Date: Fri, 31 Jul 2026 21:00:17 -0400 Subject: [PATCH 33/42] Record that apidb.indel.location is the VCF anchor base Established by reconstructing strain sequence from reference with samtools and comparing against the real consensus FASTAs: the only deletion offsets that reproduce the observed strain sequence start at location+1, never at location. Verified on two independent events (A17-10A-1 offset 1207 -> valid 1208-1212; E-1-75s-2 offset 2492 -> valid 2493-2495). This confirms rather than contradicts the < / <= asymmetry in section 5.3: an event whose location equals refStart deletes bases starting at refStart+1, so it is genuinely inside the segment and must shift the end but not the start. Also records two caveats the exercise exposed. Event position is ambiguous within repeat context (a 5-wide window of offsets reproduces the same consensus), which is the honest answer to the deferred refStart-boundary question -- an event straddling a boundary has no exact strain coordinate because the reference base does not exist in the strain. And only the net shift is meaningful: on an insertion region a clustalo alignment resolved recorded events -1,-6,+31,-1 as -1,+25,-1, identical in sum but not one-to-one. --- ...2026-07-31-strain-segment-record-design.md | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/docs/superpowers/specs/2026-07-31-strain-segment-record-design.md b/docs/superpowers/specs/2026-07-31-strain-segment-record-design.md index 9adf55d51..96ef7afd2 100644 --- a/docs/superpowers/specs/2026-07-31-strain-segment-record-design.md +++ b/docs/superpowers/specs/2026-07-31-strain-segment-record-design.md @@ -64,6 +64,36 @@ FK -> dots.nasequenceimp, study.protocolappnode strain into `study.protocolappnode`, so on the website side strain identity is one join away. This is the single most misleading thing about the table and the reason §4.1 exists. +**`location` is the VCF anchor base, not the first affected base.** Established 2026-07-31 by +reconstructing strain sequence from reference: for each event, the only deletion offsets that +reproduce the observed consensus start at `location + 1`, never at `location` itself. + +| strain | recorded offset in segment | offsets that reproduce the strain | +|---|---|---| +| A17-10A-1 | 1207 | **1208**-1212 | +| E-1-75s-2 | 2492 | **2493**-2495 | + +So a deletion covers `location+1 … location+|shift|` and `location` is the last *unaffected* +base. This **confirms** the `<` / `<=` asymmetry in §5.3: an event whose `location` equals +`refStart` deletes bases starting at `refStart+1`, i.e. genuinely inside the segment, so it +must shift the end and not the start — which is what the query does. + +Two caveats it also exposes: + +1. **Event position is ambiguous within repeat context.** Several offsets reproduce the same + consensus (1208-1212 above, a 5-wide window). VCF left-normalizes; aligners tend to place + gaps rightmost. Nothing is wrong, but a segment boundary landing inside that window inherits + the ambiguity, and no convention we control resolves it. This is the honest answer to the + deferred "refStart sits inside the segment" question: for events fully outside or fully + inside a segment the arithmetic is exact; for an event *straddling* a boundary the reference + base may not exist in the strain at all, so no exact strain coordinate exists to return. +2. **Only the net shift is meaningful, not the decomposition.** Verified on an insertion region + (A17-58A-3, ref 491000-493500): the table records `-1, -6, +31, -1` while a clustalo + alignment of the real consensus resolves the same region as `-1, +25, -1`. Both sum to + **+23**, and the recorded `-6/+31` pair 40 bp apart is one compound variant the aligner + renders as a single `+25`. The prefix sum consumes only the sum, so this is harmless -- but + do not expect a one-to-one correspondence between rows here and gaps in an alignment. + `shift` is a **signed per-event delta**, never zero: range −101..+80, with 21,945,521 negative and 21,640,063 positive rows. The strain offset at a reference position is therefore a *prefix sum*, partitioned by `(protocol_app_node_id, na_sequence_id)`. From acbc27d74c47efc8cbfabdecd4b2240fc3ab076e Mon Sep 17 00:00:00 2001 From: John Brestelli Date: Fri, 31 Jul 2026 21:15:13 -0400 Subject: [PATCH 34/42] Gate strain segments on organism, not exact sequence The EXISTS gate in StrainSegmentId.StrainSegmentsByRefSegment matched on na_sequence_id, so it did two jobs: prove strain and sequence are mutually consistent (required, and what lets this search drop the organism param) and prove indel data exists on that exact contig (not required, and wrong). Zero indels on a contig is an ordinary valid state, not missing data: the strain matches the reference there, so strain coordinates equal reference coordinates and the contig is present in the strain's consensus FASTA. Those requests returned an empty result: 689 of 6,068 valid strain/sequence pairs (11%). Surfaced by A17-10A-1 + mito_A_fumigatus_Af293, whose defline is in A17-10A-1_consensus.fa.gz. Match on taxon_id instead. The gate now proves the strain was sequenced against this organism. The substitution is exact, not approximate: (strain name, taxon) resolves to exactly one protocol app node in 452/452 pairs and no protocol_app_node_id spans more than one organism. The attribute query needs no change; its LEFT JOIN + COALESCE already yields the identity mapping. Co-Authored-By: Claude Opus 5 --- .../queries/strainSegmentQueries.xml | 45 ++++++++++--- .../plans/2026-07-31-strain-segment-record.md | 34 ++++++++++ ...2026-07-31-strain-segment-record-design.md | 67 +++++++++++++++++-- 3 files changed, 132 insertions(+), 14 deletions(-) diff --git a/Model/lib/wdk/model/questions/queries/strainSegmentQueries.xml b/Model/lib/wdk/model/questions/queries/strainSegmentQueries.xml index e379497e1..094f59fb9 100644 --- a/Model/lib/wdk/model/questions/queries/strainSegmentQueries.xml +++ b/Model/lib/wdk/model/questions/queries/strainSegmentQueries.xml @@ -9,13 +9,13 @@ 3. range - 1 <= start <= end <= ens.length (end=0 means "to end of sequence" and is normalized to ens.length in the derived table, before this gate ever sees it) - 4. EXISTS - the strain has indel data on this very sequence, matched - on na_sequence_id + 4. EXISTS - the strain has indel data somewhere on this sequence's + ORGANISM, matched on taxon_id - Indel rows recorded on a sequence make strain and sequence consistent by - construction, so gate 4 needs no separate organism input to validate - against. It is scoped by na_sequence_id, never by strain name alone: - 126 strain names map to more than one organism. + Gate 4 exists to prove strain and sequence are mutually consistent, which + is why this query needs no separate organism input to validate against. + It is scoped by taxon_id, never by strain name alone: 126 strain names map + to more than one organism. The apidb.organism join is also a gate, not just a project_id source: it is an inner join, so a sequence whose taxon_id has no apidb.organism row @@ -53,7 +53,7 @@ ELSE $$end_point_segment$$::integer END AS ref_end , ens.length::integer AS seq_length - , ens.na_sequence_id + , ens.taxon_id FROM dots.ExternalNaSequence ens , apidb.organism o WHERE o.taxon_id = ens.taxon_id @@ -78,13 +78,42 @@ -- so pan_named_ix (name, protocol_app_node_id, ...) can still be used as -- an Index Only Scan; applying a function to pan.name instead would force -- a scan of protocolappnode per row. + -- + -- The gate is on ORGANISM (ens2.taxon_id = seg.taxon_id), not on the exact + -- sequence (i.na_sequence_id = seg.na_sequence_id). What it proves is the + -- only thing this query needs: THIS STRAIN WAS SEQUENCED AGAINST THIS + -- ORGANISM, so the strain name and the reference sequence are mutually + -- consistent and the minted primary key is meaningful. What it + -- deliberately no longer proves is that the strain has indel data on this + -- exact contig -- because that is not a precondition for a valid request. + -- A strain with ZERO indels on a contig is the ordinary case of a contig + -- that simply matches the reference: the strain coordinates equal the + -- reference coordinates, the identity mapping is exactly what + -- StrainSegmentAttributes.Coords already returns via its LEFT JOIN + + -- COALESCE(..., 0), and the contig IS present in that strain's consensus + -- FASTA. Gating on the exact sequence refused 689 of the 6,068 valid + -- strain/sequence pairs in this appDb (11%), e.g. A17-10A-1 + + -- mito_A_fumigatus_Af293, whose defline >A17-10A-1_mito_A_fumigatus_Af293 + -- is right there in A17-10A-1_consensus.fa.gz. + -- + -- Organism-level scoping is safe to substitute because (strain name, + -- taxon) resolves to exactly one protocol_app_node in every one of the + -- 452 pairs present, and no protocol_app_node_id spans more than one + -- organism. The one assumption worth re-checking if the consensus + -- pipeline changes: a strain set is assumed to cover every contig of its + -- organism's reference. Af293 holds for that today (9 sequences in the + -- DB, 9 deflines in the FASTA). If a future pipeline ever omitted a + -- reference contig from the consensus, this gate would happily mint an ID + -- whose chrom has no FASTA entry. AND EXISTS ( SELECT 1 FROM apidb.indel i , study.protocolappnode pan + , dots.ExternalNaSequence ens2 WHERE i.protocol_app_node_id = pan.protocol_app_node_id - AND i.na_sequence_id = seg.na_sequence_id AND pan.name = CONCAT($$strain$$, '_Indel') + AND ens2.na_sequence_id = i.na_sequence_id + AND ens2.taxon_id = seg.taxon_id ) ]]> diff --git a/docs/superpowers/plans/2026-07-31-strain-segment-record.md b/docs/superpowers/plans/2026-07-31-strain-segment-record.md index 731b8561b..106593f36 100644 --- a/docs/superpowers/plans/2026-07-31-strain-segment-record.md +++ b/docs/superpowers/plans/2026-07-31-strain-segment-record.md @@ -498,6 +498,40 @@ project including UniDB. DynSpan duplicates its queries for UniDB precisely beca uses `@PROJECT_ID@`, which is meaningless on the portal — that is a problem this design avoids rather than a pattern to copy. +> **CORRECTION — gate 4 (the `EXISTS`) is scoped by ORGANISM, not by exact sequence.** +> Found in end-to-end QA and fixed 2026-07-31. As written below it matched +> `i.na_sequence_id = seg.na_sequence_id`, which made the gate do two jobs: prove strain +> and sequence are mutually consistent (**required** — it is what lets this search drop the +> organism param, spec §5.1.1) and prove indel data exists on that exact contig (**not +> required, and wrong**). A strain with **zero** indels on a contig is a perfectly valid +> request: it means the strain matches the reference there, so the strain coordinates +> simply equal the reference coordinates and the contig is present in that strain's +> consensus FASTA. Such requests returned `### The result is empty ###`. +> +> That refused **689 of the 6,068 valid strain/sequence pairs (11%)**. The case that +> surfaced it: `A17-10A-1` + `mito_A_fumigatus_Af293` returned empty even though +> `>A17-10A-1_mito_A_fumigatus_Af293` is present in `A17-10A-1_consensus.fa.gz`. +> +> The fix adds `ens.taxon_id` to the inner derived table and joins the `EXISTS` back +> through `dots.ExternalNaSequence ens2` on `ens2.taxon_id = seg.taxon_id`. Gate 4 now +> proves **"this strain was sequenced against this organism"**; it no longer proves "this +> strain has indel data on this exact contig". The substitution is exact rather than +> approximate: `(strain name, taxon)` resolves to exactly one protocol app node in 452/452 +> pairs, and **0** `protocol_app_node_id` values span more than one organism. Cross-organism +> requests still return zero rows. **The attribute query needs no change** — its `LEFT JOIN` +> + `COALESCE(..., 0)` already yields the identity mapping for a zero-indel segment. +> +> Assumption to re-check if the consensus pipeline changes: a strain set is assumed to +> cover **every** contig of its organism's reference (Af293: 9 sequences in the DB, 9 +> deflines in the FASTA). If that ever fails, this gate would mint an ID whose `chrom` has +> no FASTA entry. Full rationale in spec §5.1, "Gate 3 is organism-level, and that is +> deliberate" (the spec numbers this gate 3; the numbering differs, the gate is the same). +> +> Note also that the Step 1 snapshot below predates §5.1.1: the shipped query takes **no** +> `organismSinglePick` param and resolves sequences through unpartitioned +> `dots.ExternalNaSequence` + `apidb.organism` rather than `webready.GenomicSeqAttributes_p`. +> Read the committed `strainSegmentQueries.xml` as the source of truth, not this block. + **Files:** - Create: `ApiCommonModel/Model/lib/wdk/model/questions/queries/strainSegmentQueries.xml` diff --git a/docs/superpowers/specs/2026-07-31-strain-segment-record-design.md b/docs/superpowers/specs/2026-07-31-strain-segment-record-design.md index 96ef7afd2..e7b589f13 100644 --- a/docs/superpowers/specs/2026-07-31-strain-segment-record-design.md +++ b/docs/superpowers/specs/2026-07-31-strain-segment-record-design.md @@ -322,8 +322,9 @@ download: 1. the reference sequence exists — `dots.ExternalNaSequence.source_id = $$sequenceId$$`; 2. `1 <= refStart <= refEnd <= ens.length`; -3. the strain has indel data on **this** sequence — an `EXISTS` over `apidb.indel` joined to - `study.protocolappnode`, matched on `na_sequence_id`; +3. the strain has indel data somewhere on **this sequence's organism** — an `EXISTS` over + `apidb.indel` joined to `study.protocolappnode` and back to `dots.ExternalNaSequence`, + matched on `taxon_id` (**not** on `na_sequence_id`; see "Gate 3 is organism-level" below); 4. the reference sequence ID contains no `':'` — `seg.ref_seq NOT LIKE '%:%'`. Gate 4 exists because `':'` is the only delimiter in the minted primary key, so a @@ -337,10 +338,64 @@ the gate costs nothing real — it refuses 0 indel-bearing sequences and all 8 A chromosomes still pass. Gate 3 subsumes the organism check the earlier draft did explicitly: if the strain has indel -rows on that sequence, strain and sequence are consistent **by construction**. That is a -stronger guarantee than validating against a user-supplied organism, and it is what the -original requirement ("validate the reference sequence against the reference organism") -actually wanted. +rows against that sequence's organism, strain and sequence are consistent **by +construction**. That is a stronger guarantee than validating against a user-supplied +organism, and it is what the original requirement ("validate the reference sequence against +the reference organism") actually wanted. + +#### Gate 3 is organism-level, and that is deliberate + +**Corrected 2026-07-31 after end-to-end QA.** Gate 3 originally matched on +`na_sequence_id`, so it did double duty: it proved strain/sequence consistency (its job, +and the reason §5.1.1 can drop the organism param) *and* it required indel data on that +exact contig (not its job, and wrong). + +The second requirement is wrong because **zero indels on a contig is a valid, ordinary +state**, not a missing-data state. It means the strain matches the reference there: the +strain coordinates equal the reference coordinates, `StrainSegmentAttributes.Coords` +already returns exactly that identity mapping via its `LEFT JOIN` + `COALESCE(..., 0)` +(no change needed there — verified against a zero-indel PK, which yields `100-200 -> +100-200` with the correct `strain_seq_id`), and the contig **is** present in that strain's +consensus FASTA. Refusing the request produced `### The result is empty ###` for a request +the data fully supports. + +Scale of the defect: **689 of the 6,068 valid strain/sequence pairs (11%)** in `unidb_shu_a` +were refused this way. The case that surfaced it: `A17-10A-1` + `mito_A_fumigatus_Af293` +returned empty, yet `>A17-10A-1_mito_A_fumigatus_Af293` is present in +`A17-10A-1_consensus.fa.gz`. + +So the gate now matches on `ens2.taxon_id = seg.taxon_id`. Stated plainly: + +| | | +|---|---| +| What gate 3 **now proves** | this strain was sequenced against this organism, so the strain name and the reference sequence are mutually consistent and the minted PK is meaningful | +| What it **no longer proves** | this strain has indel data on this exact contig — which was never a precondition for a valid request | + +Weakening it is safe because the substitution is exact, not approximate (re-verified +against `unidb_shu_a`): + +- `(strain name, taxon)` resolves to **exactly one** `protocol_app_node` — 452 / 452 pairs, + zero exceptions. Organism scoping is therefore no less specific than node scoping. +- **0** `protocol_app_node_id` values span more than one organism. +- It remains scoped by organism, never by strain name alone: 126 strain names map to more + than one organism (§2), and the cross-organism case still returns zero rows + (`A17-10A-1` + `Pf3D7_01_v3`). +- Cost is negligible: 0.35 ms, early-exits via `indel_ix1`, no sequential scan. + +> **The one assumption a future reader must re-check.** Organism-level gating admits every +> contig of the organism, so it is sound only while **a strain set covers every contig of +> its organism's reference** — i.e. the consensus pipeline never omits a reference contig. +> That holds today: Af293 has 9 sequences in the DB and the consensus FASTA has 9 deflines, +> so the gate admits exactly the contigs that exist in the FASTA and cannot emit a `chrom` +> with no FASTA entry. If a future pipeline ever dropped a contig from the consensus, this +> gate *would* mint an ID whose `chrom` is absent from the FASTA, and gate 3 would need a +> per-strain contig manifest rather than a taxon match. + +Gate 4 becomes slightly more load-bearing under organism scoping, since it can no longer +rely on colon-free-ness being a property of the *indel-bearing* subset. It still refuses +nothing real: in `unidb_shu_a` **0** sequences reachable through gate 1+2 contain a colon +(the 10,704 figure below was measured on a different database), and all 9 Af293 contigs +pass. Strain membership itself is enforced by WDK, which validates a `flatVocabParam` value against its vocabulary — that is what makes strain names a controlled vocabulary sourced From 707b8386454fcba78c5ffa763e8e312bd900f0d4 Mon Sep 17 00:00:00 2001 From: John Brestelli Date: Fri, 31 Jul 2026 21:22:42 -0400 Subject: [PATCH 35/42] Add the shareable end-to-end test report Plain-text summary of Task 7 verification against the running site: service checks, BED coordinates for five strains on Chr1 and mito, the clustalo alignments confirming the recorded indels, the unshifted control, and the VCF-anchor-base finding. --- .../2026-07-31-strain-segment-e2e-report.txt | 123 ++++++++++++++++++ 1 file changed, 123 insertions(+) create mode 100644 docs/superpowers/2026-07-31-strain-segment-e2e-report.txt diff --git a/docs/superpowers/2026-07-31-strain-segment-e2e-report.txt b/docs/superpowers/2026-07-31-strain-segment-e2e-report.txt new file mode 100644 index 000000000..720424728 --- /dev/null +++ b/docs/superpowers/2026-07-31-strain-segment-e2e-report.txt @@ -0,0 +1,123 @@ +END-TO-END TESTING: strain-genomic-segment record (BED in strain coordinates) +FungiDB dev instance jbrestel.fungidb.org, appDb unidb_shu_a, 2026-07-31 + +WHAT WAS TESTED + +The new internal record class takes a reference-coordinate genomic location plus a +strain and returns a BED feature in that strain's consensus-sequence coordinates, +by prefix-summing the signed per-event shifts in apidb.indel. The point of the +exercise was to check the whole path against the running site rather than against +the model XML: search -> ID query -> coordinate-conversion attribute query -> BED +reporter -> a downloaded BED line, and then to check that line against the actual +strain consensus FASTA files it is meant to index into. + +Service checks passed: the record type resolves and advertises the "bed" format, the +search is correctly absent from the category tree (it is internal and has no ontology +entry), basket support is off, and a successful download leaves every error log +silent (200, report written in 7 ms, "Wrote 1 features for 1 records"). Bad input +returns an empty result rather than an error: a cross-organism sequence and an +out-of-range end both come back "### The result is empty ###". + +BED OUTPUT + +Reference Chr1_A_fumigatus_Af293:396000-399000 requested for five strains. Column 1 +is the FASTA key, column 2 is the 0-based start, column 3 the end, column 4 the +record's primary key: + + A17-10A-1_Chr1_A_fumigatus_Af293 395992 398982 A17-10A-1:Chr1_A_fumigatus_Af293:396000-399000:f 0 + + A17-3C-11_Chr1_A_fumigatus_Af293 396006 398996 A17-3C-11:Chr1_A_fumigatus_Af293:396000-399000:f 0 + + A17-58A-3_Chr1_A_fumigatus_Af293 395999 399000 A17-58A-3:Chr1_A_fumigatus_Af293:396000-399000:f 0 + + B-1-71L-1_Chr1_A_fumigatus_Af293 396035 399036 B-1-71L-1:Chr1_A_fumigatus_Af293:396000-399000:f 0 + + E-1-75s-2_Chr1_A_fumigatus_Af293 395987 398984 E-1-75s-2:Chr1_A_fumigatus_Af293:396000-399000:f 0 + + +The same reference interval maps to five different strain intervals, with different +offsets and different lengths (3001 bp in the reference; 2990, 2990, 3001, 3001 and +2997 bp in the strains). Every column-1 value was checked byte-for-byte against the +deflines in the corresponding _consensus.fa.gz and matched exactly. + +MULTIPLE SEQUENCE ALIGNMENT + +Reference substrings were pulled from dots.ExternalNaSequence and strain substrings +from the consensus FASTAs with samtools faidx, using exactly the coordinates the BED +output gave. Aligned with clustalo. + +Chr1, reference 396000-399000. Alignment length 3001 - the same as the reference - +so no strain required an insertion here: + + sequence gaps identity vs reference + REFERENCE 0 100.000% + A17-10A-1 11 99.799% + A17-3C-11 11 99.799% + A17-58A-3 0 99.867% + B-1-71L-1 0 99.667% + E-1-75s-2 4 99.766% + + columns 1196-1240, the 11 bp deletion: + REFERENCE GTGGTATTGCTTCTTCCATCAAACTTCTACAGAGCACAGCGGCTA + A17-10A-1 GTGGTATTGCTTCTT-----------CTACAGAGCACAGCGGCTA + A17-3C-11 GTGGTATTGCTTCTT-----------CTACAGAGCACAGCGGCTA + A17-58A-3 GTGGTATTGCTTCTTCCATCAAACTTCTACAGAGCACAGCGGCTA + B-1-71L-1 GTGGTATTGCTTCTTCCATCAAACTTCTACAGAGCACMGCGGCTA + E-1-75s-2 GTGGTATTGCTTCTTCCATCAAACTTCTACAGAGCACCGCGGCTA + +The gaps reproduce the recorded indels exactly. apidb.indel holds one event for +A17-10A-1 and A17-3C-11 in this window (location 397206, shift -11) and one for +E-1-75s-2 (location 398491, shift -4); the alignment shows an 11 bp gap for the first +two, a 4 bp gap for the third, and no gap at all for the two strains with no recorded +events. Gap sizes match the recorded shifts exactly. Gap positions differ by 3-4 bp +because indel placement is ambiguous inside a repeat - VCF left-normalises, aligners +place gaps rightmost - and both descriptions produce the same sequence. + +mito, reference 500-2000. No strain has any indel before position 2000 on mito, so +all five map to the identical interval 500-2000, and the alignment is the expected +degenerate case: length 1501, zero gaps anywhere, four of five strains 100.000% +identical to the reference and the fifth differing by a single SNP. + + sequence gaps identity vs reference + REFERENCE 0 100.000% + A17-10A-1 0 100.000% + A17-3C-11 0 100.000% + A17-58A-3 0 100.000% + B-1-71L-1 0 100.000% + E-1-75s-2 0 99.933% + + columns 401-460: + REFERENCE TCTAGCATTAATGGTTATGCCTATAACTGATTTATCTAAATTAAGAGGAGTACAGTTCAG + A17-10A-1 TCTAGCATTAATGGTTATGCCTATAACTGATTTATCTAAATTAAGAGGAGTACAGTTCAG + A17-3C-11 TCTAGCATTAATGGTTATGCCTATAACTGATTTATCTAAATTAAGAGGAGTACAGTTCAG + A17-58A-3 TCTAGCATTAATGGTTATGCCTATAACTGATTTATCTAAATTAAGAGGAGTACAGTTCAG + B-1-71L-1 TCTAGCATTAATGGTTATGCCTATAACTGATTTATCTAAATTAAGAGGAGTACAGTTCAG + E-1-75s-2 TCTAGCATTAATGGTTATGCCTATAACTGATTTATCTAAATTAAGAGGAGTACAGTTCAG + +mito is worth reporting because it was broken until today. The search originally +required a strain to have indel data on the exact requested sequence, so a contig a +strain simply matches - like A17-10A-1 on mito - returned no result at all, even +though the contig is present in that strain's FASTA. That refused 689 of 6,068 valid +strain/sequence pairs (11%). The check is now scoped to the organism rather than the +sequence, which still proves the strain and sequence are consistent while allowing +the identity mapping. A17-10A-1's mito is also exactly the reference length (31,765 +bp), independently consistent with its having no indels there. + +A CONTROL, AND ONE CORRECTION + +To confirm the coordinate conversion does real work, the strain sequence was also +extracted at the unshifted reference coordinates. Against reference, the shifted +coordinates give a 506 bp exact match before the first SNP and 99.92% identity up to +the first internal indel; the unshifted control gives a 0 bp prefix and 25.0% +identity, i.e. chance. Splitting at the one known indel and realigning by the +recorded -11 restores 99.66% identity on the far side. + +One finding came out of this that is worth recording: apidb.indel.location is the VCF +anchor base, not the first affected base. Reconstructing the strain sequence showed +that the only deletion offsets reproducing the observed consensus start at +location + 1, never at location itself (A17-10A-1: recorded 1207, valid 1208-1212; +E-1-75s-2: recorded 2492, valid 2493-2495). This confirms rather than contradicts the +boundary handling in the query. It also means an event straddling a segment boundary +has no exact strain coordinate, because the reference base does not exist in the +strain - a genuine limit of the data, not of the implementation. + +Insertions were checked too. A17-58A-3 on Chr1 491000-493500 yields a strain segment +23 bp longer than the reference. The table records four events there (-1, -6, +31, -1) +and the alignment resolves the same region as (-1, +25, -1). Both sum to +23; the +-6/+31 pair 40 bp apart is one compound variant the aligner renders as a single +insertion. Only the net shift is meaningful, which is all the conversion uses. From f05574eeb661ab224582de0fa5059ff2da5cb2bf Mon Sep 17 00:00:00 2001 From: John Brestelli Date: Fri, 31 Jul 2026 21:25:35 -0400 Subject: [PATCH 36/42] Drop apidb.indel coordinates from the shareable test report The report's audience reads the alignment against the reference and never sees apidb.indel, so the 3-4 bp difference between a recorded indel location and the aligned gap position is internal noise that reads as an unexplained discrepancy. Removed it and the anchor-base derivation, keeping the parts that describe the output: gap sizes match the recorded shifts, and a segment boundary falling inside a deletion has no exact strain coordinate. The full derivation stays in the design spec, where it belongs. --- .../2026-07-31-strain-segment-e2e-report.txt | 25 ++++++++----------- 1 file changed, 10 insertions(+), 15 deletions(-) diff --git a/docs/superpowers/2026-07-31-strain-segment-e2e-report.txt b/docs/superpowers/2026-07-31-strain-segment-e2e-report.txt index 720424728..a32655852 100644 --- a/docs/superpowers/2026-07-31-strain-segment-e2e-report.txt +++ b/docs/superpowers/2026-07-31-strain-segment-e2e-report.txt @@ -60,13 +60,11 @@ so no strain required an insertion here: B-1-71L-1 GTGGTATTGCTTCTTCCATCAAACTTCTACAGAGCACMGCGGCTA E-1-75s-2 GTGGTATTGCTTCTTCCATCAAACTTCTACAGAGCACCGCGGCTA -The gaps reproduce the recorded indels exactly. apidb.indel holds one event for -A17-10A-1 and A17-3C-11 in this window (location 397206, shift -11) and one for -E-1-75s-2 (location 398491, shift -4); the alignment shows an 11 bp gap for the first -two, a 4 bp gap for the third, and no gap at all for the two strains with no recorded -events. Gap sizes match the recorded shifts exactly. Gap positions differ by 3-4 bp -because indel placement is ambiguous inside a repeat - VCF left-normalises, aligners -place gaps rightmost - and both descriptions produce the same sequence. +The gaps reproduce the recorded indels exactly. Two strains carry an 11 bp deletion in +this window and one carries a 4 bp deletion; the remaining two have no recorded event +and show no gap at all. Every gap size matches the recorded shift exactly, and the +gaps fall where the alignment against the reference puts them, which is the view that +matters when reading the strain sequence against the reference. mito, reference 500-2000. No strain has any indel before position 2000 on mito, so all five map to the identical interval 500-2000, and the alignment is the expected @@ -107,14 +105,11 @@ the first internal indel; the unshifted control gives a 0 bp prefix and 25.0% identity, i.e. chance. Splitting at the one known indel and realigning by the recorded -11 restores 99.66% identity on the far side. -One finding came out of this that is worth recording: apidb.indel.location is the VCF -anchor base, not the first affected base. Reconstructing the strain sequence showed -that the only deletion offsets reproducing the observed consensus start at -location + 1, never at location itself (A17-10A-1: recorded 1207, valid 1208-1212; -E-1-75s-2: recorded 2492, valid 2493-2495). This confirms rather than contradicts the -boundary handling in the query. It also means an event straddling a segment boundary -has no exact strain coordinate, because the reference base does not exist in the -strain - a genuine limit of the data, not of the implementation. +One limitation is worth stating plainly, because it is a property of the data rather +than of the implementation. If a requested segment boundary falls inside a deletion, +that reference base does not exist in the strain at all, so there is no exact strain +coordinate to return for it. Segments whose boundaries sit outside any deletion - the +ordinary case, and all of the cases above - convert exactly. Insertions were checked too. A17-58A-3 on Chr1 491000-493500 yields a strain segment 23 bp longer than the reference. The table records four events there (-1, -6, +31, -1) From cf8c49cbab25e5e4568b15881330691d5ee0d9d7 Mon Sep 17 00:00:00 2001 From: John Brestelli Date: Fri, 31 Jul 2026 21:36:36 -0400 Subject: [PATCH 37/42] Stamp provenance on the strain-segment numbers and correct the wrong ones Documentation-only pass over the organism-scoping change; no SQL semantics touched. Two code reviews agreed the SQL is right and every finding was a number or a comment that described a different database. - Separate unidb_shu_a (live appDb) from genomicsdb_rebuild01 everywhere the two were conflated: strain-name ambiguity (0 of 452 live vs 126 on rebuild01), colon-bearing source_ids (0 of 160,581 vs 10,704), indel-bearing sequences (120 vs 20,823), row counts (1,855,449 / 1,356 vs 43.6M / 61,512). - Enumerate all FIVE ID-query gates, in the spec's numbering, in both the spec and the XML header, so a cited "gate 3" means the same gate in both. - Measure and record the REJECT path, which the old "0.35 ms, no sequential scan" claim omitted: ~101 ms vs ~53 ms for the largest strain, scaling with the strain's indel row count. Record the rejected LIMIT 1 rewrite and why (it trades slow for silently wrong). - Replace the Af293-only safety evidence with a three-organism check including TREU927, where 144 newly-admitted pairs sit on 36 never-indeled contigs and all 131 deflines match the DB set exactly. - Reframe the residual assumption as set equality that can drift from the DB side too, and add it to the section 10 risk register. - Fix the stale param comment describing the removed exact-sequence gate, the "8 Af293 chromosomes" (it is 9), and a below/above cross-reference. Co-Authored-By: Claude Opus 5 --- .../questions/params/strainSegmentParams.xml | 22 ++- .../queries/strainSegmentQueries.xml | 103 ++++++++---- .../records/strainSegmentAttributeQueries.xml | 38 +++-- ...2026-07-31-strain-segment-record-design.md | 152 ++++++++++++++---- 4 files changed, 236 insertions(+), 79 deletions(-) diff --git a/Model/lib/wdk/model/questions/params/strainSegmentParams.xml b/Model/lib/wdk/model/questions/params/strainSegmentParams.xml index e231031dc..2471568f3 100644 --- a/Model/lib/wdk/model/questions/params/strainSegmentParams.xml +++ b/Model/lib/wdk/model/questions/params/strainSegmentParams.xml @@ -23,18 +23,28 @@ + regexp_replace no longer applies here. + + Row-count provenance, because two databases get confused here. The 61,512 + protocolappnode rows and 43.6M apidb.indel rows quoted by the original of + this comment were measured on genomicsdb_rebuild01, a DIFFERENT and larger + database. On unidb_shu_a (the live appDb, measured 2026-07-31) it is 1,356 + protocolappnode rows and 1,855,449 indel rows, of which 452 nodes carry the + '_Indel' suffix. The shape argument above holds at either scale; only the + magnitude of the win changes. --> diff --git a/Model/lib/wdk/model/questions/queries/strainSegmentQueries.xml b/Model/lib/wdk/model/questions/queries/strainSegmentQueries.xml index 094f59fb9..ed09bb24f 100644 --- a/Model/lib/wdk/model/questions/queries/strainSegmentQueries.xml +++ b/Model/lib/wdk/model/questions/queries/strainSegmentQueries.xml @@ -2,23 +2,34 @@ - + multiPick="true" + maxSelectedCount="500"> - Strain or isolate whose coordinate system the returned segment is expressed in. - The list is all strains that have indel data loaded. + One or more strains or isolates whose coordinate systems the returned segments + are expressed in: the search returns one segment per selected strain. The list is + all strains that have indel data loaded. A selected strain that does not belong to + the reference sequence's organism is dropped from the result rather than being an + error. diff --git a/Model/lib/wdk/model/questions/queries/strainSegmentQueries.xml b/Model/lib/wdk/model/questions/queries/strainSegmentQueries.xml index ed09bb24f..3dfbe0f48 100644 --- a/Model/lib/wdk/model/questions/queries/strainSegmentQueries.xml +++ b/Model/lib/wdk/model/questions/queries/strainSegmentQueries.xml @@ -2,7 +2,16 @@ -