Skip to content

feat(lapis): add sequence position fields syntax to aggregated endpoint #1768

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
fhennig wants to merge 8 commits into
base: main
Choose a base branch
from
Open

feat(lapis): add sequence position fields syntax to aggregated endpoint #1768

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
0e9b00c
feat(lapis): add sequence position fields syntax to aggregated endpoint
fhennig Jul 14, 2026
15df660
feat(lapis): return position fields under user-facing name in response
fhennig Jul 14, 2026
18eaccb
feat(lapis): use isSingleSegmented() for shorthand syntax; echo [N] b…
fhennig Jul 14, 2026
88d9562
refactor(lapis): use quoted identifiers in SaneQL; simplify position …
fhennig Jul 15, 2026
bf2554e
chore(lapis): fix ktlint warnings (redundant braces, import ordering)
fhennig Jul 15, 2026
08df65e
Potential fix for pull request finding
fhennig Jul 15, 2026
097df7d
fix(lapis): close unterminated string literal in ControllerDescriptio…
fhennig Jul 15, 2026
2c4d644
fix(lapis): reject sequence position fields in /details endpoint
fhennig Jul 15, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion lapis/src/main/kotlin/org/genspectrum/lapis/controller/ControllerDescriptions.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,11 @@ such as combinations of mutations ("mutation 1 or mutation 2") and other filters
const val AGGREGATED_GROUP_BY_FIELDS_DESCRIPTION =
"""The fields to stratify by.
If empty, only the overall count is returned.
If requesting CSV or TSV data, the columns are ordered in the same order as the fields are specified here."""
If requesting CSV or TSV data, the columns are ordered in the same order as the fields are specified here.

Sequence positions can be requested using the syntax `SequenceName[position]` (e.g. `S[501]` for position 501 of sequence `S`).
For single-segmented genomes, the shorthand `[position]` (e.g. `[501]`) can be used.
Position field column names in the response use the canonical sequence name from the reference genome (case-insensitive input), e.g. `s[501]` -> `S[501]`."""
Comment on lines +73 to +75
Comment on lines +73 to +75

@fengelniederhammer fengelniederhammer Jul 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We should probably also add something in the llms.txt?

Comment on lines +73 to +75

@fengelniederhammer fengelniederhammer Jul 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Usually we tried to avoid those formulations (if single segmented ...). Instead, we generate them at run time: then we know in which case we are.

const val AGGREGATED_ORDER_BY_FIELDS_DESCRIPTION =
"""The fields of the response to order by.
Fields specified here must either be \"count\" or also be present in \"fields\".
Expand Down
15 changes: 12 additions & 3 deletions lapis/src/main/kotlin/org/genspectrum/lapis/controller/LapisController.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@ import org.genspectrum.lapis.request.AminoAcidInsertion
import org.genspectrum.lapis.request.AminoAcidMutation
import org.genspectrum.lapis.request.CaseInsensitiveFieldConverter
import org.genspectrum.lapis.request.DEFAULT_MIN_PROPORTION
import org.genspectrum.lapis.request.Field
import org.genspectrum.lapis.request.GetRequestSequenceFilters
import org.genspectrum.lapis.request.MRCASequenceFiltersRequest
import org.genspectrum.lapis.request.MutationProportionsRequest
Expand All @@ -74,6 +75,7 @@ import org.genspectrum.lapis.request.SPECIAL_REQUEST_PROPERTIES
import org.genspectrum.lapis.request.SequenceFiltersRequest
import org.genspectrum.lapis.request.SequenceFiltersRequestWithFields
import org.genspectrum.lapis.request.SequenceFiltersRequestWithGenes
import org.genspectrum.lapis.request.SequencePositionField
import org.genspectrum.lapis.request.toOrderBySpec
import org.genspectrum.lapis.request.validatePhyloTreeField
import org.genspectrum.lapis.response.AggregatedCollection
Expand Down Expand Up @@ -368,10 +370,10 @@ class LapisController(
)
}

private fun getAggregatedCollection(request: SequenceFiltersRequestWithFields): AggregatedCollection =
private fun getAggregatedCollection(request: SequenceFiltersRequestWithFields) =
AggregatedCollection(
records = siloQueryModel.getAggregated(request),
fields = request.fields.map { it.fieldName },
fields = request.fields.map { it.outputColumnName },
)

@GetMapping(NUCLEOTIDE_MUTATIONS_ROUTE, produces = [MediaType.APPLICATION_JSON_VALUE])
Expand Down Expand Up @@ -1490,7 +1492,14 @@ class LapisController(
}

private fun getDetailsCollection(request: SequenceFiltersRequestWithFields): DetailsCollection {
val fields = request.fields.map { it.fieldName }
val positionFields = request.fields.filterIsInstance<SequencePositionField>()
if (positionFields.isNotEmpty()) {
throw BadRequestException(
"Sequence position fields are not supported for this endpoint: " +
positionFields.joinToString(", ") { it.userFacingName },
)
Comment on lines +1497 to +1500

@fengelniederhammer fengelniederhammer Jul 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I find it a bit weird to first allow a SequencePositionField while parsing and then reject it. Wouldn't it be better to differentiate already in the types that the controller methods accept?

}
val fields = request.fields.filterIsInstance<Field>().map { it.fieldName }

Comment thread
fhennig marked this conversation as resolved.
return DetailsCollection(
records = siloQueryModel.getDetails(request),
Expand Down
13 changes: 8 additions & 5 deletions lapis/src/main/kotlin/org/genspectrum/lapis/model/SiloQueryModel.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,15 @@ package org.genspectrum.lapis.model
import org.genspectrum.lapis.config.DatabaseConfig
import org.genspectrum.lapis.config.ReferenceGenomeSchema
import org.genspectrum.lapis.request.CommonSequenceFilters
import org.genspectrum.lapis.request.Field
import org.genspectrum.lapis.request.MRCASequenceFiltersRequest
import org.genspectrum.lapis.request.MutationProportionsRequest
import org.genspectrum.lapis.request.MutationsField
import org.genspectrum.lapis.request.OrderBySpec
import org.genspectrum.lapis.request.PhyloTreeSequenceFiltersRequest
import org.genspectrum.lapis.request.SequenceFiltersRequest
import org.genspectrum.lapis.request.SequenceFiltersRequestWithFields
import org.genspectrum.lapis.request.SequencePositionField
import org.genspectrum.lapis.response.ExplicitlyNullable
import org.genspectrum.lapis.response.InfoData
import org.genspectrum.lapis.response.InsertionResponse
Expand Down Expand Up @@ -38,10 +40,11 @@ class SiloQueryModel(
siloClient.sendQuery(
SiloQuery(
SiloAction.aggregated(
sequenceFilters.fields.map { it.fieldName },
sequenceFilters.orderByFields,
sequenceFilters.limit,
sequenceFilters.offset,
groupByFields = sequenceFilters.fields.filterIsInstance<Field>().map { it.fieldName },
orderByFields = sequenceFilters.orderByFields,
limit = sequenceFilters.limit,
offset = sequenceFilters.offset,
sequencePositionFields = sequenceFilters.fields.filterIsInstance<SequencePositionField>(),
),
siloFilterExpressionMapper.map(sequenceFilters),
),
Expand Down Expand Up @@ -140,7 +143,7 @@ class SiloQueryModel(
siloClient.sendQuery(
SiloQuery(
SiloAction.details(
sequenceFilters.fields.map { it.fieldName }.ifEmpty { allMetadataFields },
sequenceFilters.fields.filterIsInstance<Field>().map { it.fieldName }.ifEmpty { allMetadataFields },
sequenceFilters.orderByFields,
sequenceFilters.limit,
sequenceFilters.offset,
Expand Down
79 changes: 70 additions & 9 deletions lapis/src/main/kotlin/org/genspectrum/lapis/request/Field.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,32 @@
package org.genspectrum.lapis.request

import org.genspectrum.lapis.config.DatabaseConfig
import org.genspectrum.lapis.config.ReferenceGenomeSchema
import org.genspectrum.lapis.controller.BadRequestException
import org.springframework.stereotype.Component

private val SEQUENCE_POSITION_REGEX = Regex("""^([A-Za-z][A-Za-z0-9_]*)\[(\d+)\]$""")
private val SHORTHAND_POSITION_REGEX = Regex("""^\[(\d+)\]$""")
Comment thread
fengelniederhammer marked this conversation as resolved.
Comment on lines +8 to +9

@fengelniederhammer fengelniederhammer Jul 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
private val SEQUENCE_POSITION_REGEX = Regex("""^([A-Za-z][A-Za-z0-9_]*)\[(\d+)\]$""")
private val SHORTHAND_POSITION_REGEX = Regex("""^\[(\d+)\]$""")
private val SEQUENCE_POSITION_REGEX = Regex("""^([A-Za-z][A-Za-z0-9_]*)\[(\d+)]$""")
private val SHORTHAND_POSITION_REGEX = Regex("""^\[(\d+)]$""")

apparently those closing brackets don't need to be escaped.


sealed interface RequestField {
val outputColumnName: String
}

data class Field(
val fieldName: String,
)
) : RequestField {
override val outputColumnName: String get() = fieldName
}

data class SequencePositionField(
val sequenceName: String,
val position: Int,
val isSingleSegment: Boolean = false,
) : RequestField {
/** Name used both as the SaneQL alias and as the response column key, e.g. `S[501]` or `[501]` for shorthand. */
val userFacingName: String get() = if (isSingleSegment) "[$position]" else "$sequenceName[$position]"
override val outputColumnName: String get() = userFacingName
Comment thread
fhennig marked this conversation as resolved.
}
Comment on lines +21 to +29

@fengelniederhammer fengelniederhammer Jul 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
data class SequencePositionField(
val sequenceName: String,
val position: Int,
val isSingleSegment: Boolean = false,
) : RequestField {
/** Name used both as the SaneQL alias and as the response column key, e.g. `S[501]` or `[501]` for shorthand. */
val userFacingName: String get() = if (isSingleSegment) "[$position]" else "$sequenceName[$position]"
override val outputColumnName: String get() = userFacingName
}
data class SequencePositionField(
val sequenceName: String?,
val position: Int,
) : RequestField {
override val outputColumnName: String = if (sequenceName == null) {
"[$position]"
} else {
"$sequenceName[$position]"
}
}

We can encode isSingleSegment into sequenceName by making it nullable.


fun interface FieldConverter<T> {
fun convert(source: String): T
Expand All @@ -17,25 +37,61 @@ fun interface FieldConverter<T> {
@Component
class CaseInsensitiveFieldConverter(
private val caseInsensitiveFieldsCleaner: CaseInsensitiveFieldsCleaner,
) : FieldConverter<Field> {
override fun convert(source: String): Field {
private val referenceGenomeSchema: ReferenceGenomeSchema,
) : FieldConverter<RequestField> {
override fun convert(source: String): RequestField {
val shorthandMatch = SHORTHAND_POSITION_REGEX.matchEntire(source)
if (shorthandMatch != null) {
val position = shorthandMatch.groupValues[1].toIntOrNull()
?: throw BadRequestException("Invalid position in '$source': must be a positive integer")
if (position <= 0) {
throw BadRequestException("Invalid position in '$source': must be a positive integer, got $position")
}
if (!referenceGenomeSchema.isSingleSegmented()) {
throw BadRequestException(
"Shorthand position syntax '[N]' can only be used for single-segmented genomes",
)
}
val canonicalName = referenceGenomeSchema.nucleotideSequences.first().name
return SequencePositionField(canonicalName, position, isSingleSegment = true)
}

val positionMatch = SEQUENCE_POSITION_REGEX.matchEntire(source)
if (positionMatch != null) {
val name = positionMatch.groupValues[1]
val position = positionMatch.groupValues[2].toIntOrNull()
?: throw BadRequestException("Invalid position in '$source': must be a positive integer")
if (position <= 0) {
throw BadRequestException("Invalid position in '$source': must be a positive integer, got $position")
}
val canonicalName = referenceGenomeSchema.getSequenceNameFromCaseInsensitiveName(name)
?: throw BadRequestException(
"Unknown sequence '$name' in '$source', known sequences are: " +
(referenceGenomeSchema.getNucleotideSequenceNames() + referenceGenomeSchema.getGeneNames())
.joinToString(", "),
)
return SequencePositionField(canonicalName, position)
}
Comment on lines +43 to +74

@fengelniederhammer fengelniederhammer Jul 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could we collapse those two branches by making the sequence name in the regex optional? The position parsing is the same, and we could streamline the sequence name parsing.


val cleaned = caseInsensitiveFieldsCleaner.clean(source)
?: throw BadRequestException(
"Unknown field: '$source', known values are ${caseInsensitiveFieldsCleaner.getKnownFields()}",
)

return Field(cleaned)
}

override fun validatePhyloTreeFields(source: String): Field {
override fun validatePhyloTreeFields(source: String): RequestField {

@fengelniederhammer fengelniederhammer Jul 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Isn't this function actually duplicate? Below, we have another validatePhyloTreeField that looks like basically the same.

@fengelniederhammer fengelniederhammer Jul 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think this one isn't even used anywhere...

val converted = convert(source)
if (converted !is Field) {
throw BadRequestException(
"Position fields like '$source' cannot be used as phylo tree fields",
)
}
val validFields = caseInsensitiveFieldsCleaner.getPhyloTreeFields()
if (converted.fieldName !in validFields) {
throw BadRequestException(
"Field '${converted.fieldName}' is not a phylo tree field, " +
"known phylo tree fields are [${validFields.joinToString(
", ",
)}]",
"known phylo tree fields are [${validFields.joinToString(", ")}]",
)
}
return converted
Expand All @@ -44,10 +100,15 @@ class CaseInsensitiveFieldConverter(

fun validatePhyloTreeField(
source: String,
fieldConverter: FieldConverter<Field>,
fieldConverter: FieldConverter<RequestField>,
databaseConfig: DatabaseConfig,
): Field {
val converted = fieldConverter.convert(source)
if (converted !is Field) {
throw BadRequestException(
"Position fields like '$source' cannot be used as phylo tree fields",
)
}
val validFields = databaseConfig.schema.metadata.filter { it.isPhyloTreeField }.map { it.name }
if (converted.fieldName !in validFields) {
throw BadRequestException(
Expand Down
2 changes: 1 addition & 1 deletion lapis/src/main/kotlin/org/genspectrum/lapis/request/PhyloTreeSequenceFiltersRequest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ class MRCASequenceFiltersRequestDeserializer(

fun parsePhyloTreeProperty(
node: JsonNode,
fieldConverter: FieldConverter<Field>,
fieldConverter: FieldConverter<RequestField>,
databaseConfig: DatabaseConfig,
): Field {
val phyloTreeField = node.get(PHYLO_TREE_FIELD_PROPERTY)
Expand Down
2 changes: 1 addition & 1 deletion lapis/src/main/kotlin/org/genspectrum/lapis/request/SequenceFiltersRequestWithFields.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ data class SequenceFiltersRequestWithFields(
override val aminoAcidMutations: List<AminoAcidMutation>,
override val nucleotideInsertions: List<NucleotideInsertion>,
override val aminoAcidInsertions: List<AminoAcidInsertion>,
val fields: List<Field>,
val fields: List<RequestField>,
override val orderByFields: OrderBySpec = OrderBySpec.EMPTY,
override val limit: Int? = null,
override val offset: Int? = null,
Expand Down
4 changes: 2 additions & 2 deletions lapis/src/main/kotlin/org/genspectrum/lapis/silo/SaneQlAst.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,12 +70,12 @@ data class SaneQlList(
override fun render() = "{" + items.joinToString(", ") { it.render() } + "}"
}

/** A `name:=value` assignment, used inside [SaneQlList]s that act as records, e.g. `{count:=count()}`. */
/** A `"name":=value` assignment, used inside [SaneQlList]s that act as records, e.g. `{"count":=count()}`. */
data class SaneQlAssignment(
val name: String,
val value: SaneQlExpression,
) : SaneQlExpression {
override fun render() = "$name:=${value.render()}"
override fun render() = "\"${name.replace("\"", "\"\"")}\":=${value.render()}"
}

/** `"column" = value` */
Expand Down
51 changes: 40 additions & 11 deletions lapis/src/main/kotlin/org/genspectrum/lapis/silo/SiloQuery.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import com.fasterxml.jackson.annotation.JsonProperty
import org.genspectrum.lapis.request.Order
import org.genspectrum.lapis.request.OrderByField
import org.genspectrum.lapis.request.OrderBySpec
import org.genspectrum.lapis.request.SequencePositionField
import org.genspectrum.lapis.response.AggregationData
import org.genspectrum.lapis.response.DetailsData
import org.genspectrum.lapis.response.InsertionData
Expand Down Expand Up @@ -87,9 +88,11 @@ sealed class SiloAction<ResponseType>(
orderByFields: OrderBySpec = OrderBySpec.EMPTY,
limit: Int? = null,
offset: Int? = null,
sequencePositionFields: List<SequencePositionField> = emptyList(),
): SiloAction<AggregationData> =
AggregatedAction(
groupByFields = groupByFields,
sequencePositionFields = sequencePositionFields,
orderByFields = getOrderByFieldsList(orderByFields),
randomize = getRandomize(orderByFields),
limit = limit,
Expand Down Expand Up @@ -224,6 +227,7 @@ sealed class SiloAction<ResponseType>(
@JsonInclude(JsonInclude.Include.NON_EMPTY)
data class AggregatedAction(
val groupByFields: List<String>,
val sequencePositionFields: List<SequencePositionField> = emptyList(),
override val orderByFields: List<OrderByField> = emptyList(),
override val randomize: RandomizeConfig? = null,
override val limit: Int? = null,
Expand All @@ -235,17 +239,42 @@ sealed class SiloAction<ResponseType>(
val type: String = "Aggregated"

override fun ownSaneQlSteps() =
listOf(
SaneQlStep(
"groupBy",
positionalArgs = buildList {
add(SaneQlList(listOf(SaneQlAssignment("count", SaneQlFunctionCall("count")))))
if (groupByFields.isNotEmpty()) {
add(SaneQlList(groupByFields.map { id(it) }))
}
},
),
)
buildList {
if (sequencePositionFields.isNotEmpty()) {
add(
SaneQlStep(
"map",
positionalArgs = listOf(
SaneQlList(
sequencePositionFields.map { field ->
SaneQlAssignment(
field.userFacingName,
SaneQlMethodCall(
SaneQlIdentifier(field.sequenceName),
"at",
listOf(SaneQlInt(field.position)),
),
)
Comment on lines +250 to +257

@fengelniederhammer fengelniederhammer Jul 16, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do we need to deduplicate those somewhere?

},
),
),
),
)
}
val allGroupByColumns =
groupByFields.map { id(it) } + sequencePositionFields.map { id(it.userFacingName) }
add(
SaneQlStep(
"groupBy",
positionalArgs = buildList {
add(SaneQlList(listOf(SaneQlAssignment("count", SaneQlFunctionCall("count")))))
if (allGroupByColumns.isNotEmpty()) {
add(SaneQlList(allGroupByColumns))
}
},
),
)
}
}

@JsonInclude(JsonInclude.Include.NON_EMPTY)
Expand Down
22 changes: 22 additions & 0 deletions lapis/src/test/kotlin/org/genspectrum/lapis/controller/LapisControllerTest.kt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -533,6 +533,28 @@ class LapisControllerTest(
.andExpect(jsonPath("\$.data[0].country").value("Switzerland"))
.andExpect(jsonPath("\$.data[0].date").value("a date"))
}

@Test
fun `GET details with a sequence position field returns bad request`() {
mockMvc.perform(
getSample(DETAILS_ROUTE)
.queryParam("country", "Switzerland")
.queryParam("fields", "gene1[501]"),
)
.andExpect(status().isBadRequest)
.andExpect(jsonPath("\$.error.detail").value(containsString("Sequence position fields are not supported")))
}

@Test
fun `POST JSON details with a sequence position field returns bad request`() {
mockMvc.perform(
postSample(DETAILS_ROUTE)
.content("""{"country": "Switzerland", "fields": ["gene1[501]"]}""")
.contentType(MediaType.APPLICATION_JSON),
)
.andExpect(status().isBadRequest)
.andExpect(jsonPath("\$.error.detail").value(containsString("Sequence position fields are not supported")))
}
}

fun getSample(path: String): MockHttpServletRequestBuilder = get("/sample/$path")
Expand Down
Loading
Loading