ukaea / ukaea/fds

Catalogue search: filter shots and datasets by scientific metadata and feature annotations

Open
#53 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

api architecture feature metadata priority: high
Dominant language
Python
Stars
0
Forks
1
Avg merge
26m
Merged PRs (30d)
24

Description

Summary

Every list endpoint takes offset/limit and nothing else. There is no filter on any field, so
catalogue-level questions cannot be answered server-side:

  • "MAST shots that disrupted"
  • "equilibrium Datasets from ELMy MAST-U shots"
  • "all thomson_positions geometry versions valid during 2008" (ADR-0036)
  • "all H-mode shots above 1 MA" (ADR-0031, the motivating example)

The facts are already recorded. ADR-0031 (scientific metadata), ADR-0035 (feature annotation) and
ADR-0043 (annotation datasets) have all landed the model, and the demo seeds disruption,
confinement_mode, elm and mode annotations on MAST 30421. Nothing can query them; a client must page
the whole collection and filter locally.

This issue is the missing referent for the search deferrals in ADR-0035, ADR-0036 and ADR-0043,
which previously pointed at #40 (schema-driven JSON-LD projection, unrelated).

Scope

Server-side filtering on the Shot and Dataset list endpoints:

  • Feature annotations and scientific metadata: presence (has a property named X) and equality
    (X = value). Extent-range predicates ("flat-tops longer than 1 s") are a later step, not
    required for the first cut.
  • Indexed scalar columns already available: dataset name, level, quality_flag, version.
  • Enough composition to answer the two-hop case (filter shots by annotation, then their datasets by name)
    in one request rather than N+1.

Sequencing and blockers

  1. Filter syntax is an unmade decision and needs its own ADR. Repeated ?annotation= params vs. a
    mini-grammar; how value equality and, later, extent ranges are expressed. ADR numbers 0040 and
    0042 are free. Start narrow.

  2. scientific_metadata is Column(JSON), not JSONB. On Postgres, SQLModel's JSON maps to
    json, which takes no GIN index and no containment operator, so an annotation filter is a sequential
    scan with a per-row parse. Search that scales therefore depends on #43 (ADR-0033). Search that
    works does not: SQLite's json_each can back a correct implementation now, which is a cheap
    way to validate the syntax before committing to an index design. Same applies to applies_to
    (ADR-0036) and the other JSON columns.

  3. Pagination and access control run in the wrong order. shot_service.get_multi_by_device_name
    (app/services/shot_service.py:223) pages in SQL then filters by access in Python; the Dataset
    services do the same. A page of 100 can return 40 rows, and there is no consistent way to page.
    Tolerable while listing everything, wrong under search, because "how many MAST shots disrupted?"
    returns a wrong count. The policy predicate needs folding into the query, which is not trivial
    given inheritance from device and shot policies. Prerequisite, not follow-up.

  4. No cross-device shot listing. Only /devices/{device_name}/shots exists. Both examples above
    name a device so it is not blocking, but "all shots that disrupted" needs a global route.

Open question: how a repeated annotation name combines (amends ADR-0044)

ADR-0044 settles this for repeated parameters: they AND together, and there is no OR form, on the
grounds that a caller wanting a union can issue two requests. That decision stands. What follows is
not a gap in it but an interaction it did not consider: its worked example repeats different names
(disruption and elm), where AND is plainly right, and the shipped behaviour for a repeated
name does not follow from that example.

Each ?annotation= term becomes an independent EXISTS over the JSON array, and the terms are
ANDed. Measured against a fixture where shot 1 has elm=type-I, shot 2 has elm=type-III, shot 3
carries both entries, and shot 4 has elm with no value:

elm                       -> 1, 2, 3, 4
elm:type-I                -> 1, 3
elm + elm:type-I          -> 1, 3      (identical to elm:type-I on its own)
elm:type-I + elm:type-III -> 3         (only the shot carrying both)

Two consequences:

  • Mixing presence and equality on one name is a silent no-op. EXISTS(name='elm') is implied by
    EXISTS(name='elm' AND value='type-I'), so the bare term constrains nothing.
  • Repeating a name with different values means "carried all of these", not "carried any of these".
    A request for type-I or type-III ELMs returns only the shots that had both, excluding the shots
    the question is usually after. There is currently no way to express OR.

AND is clearly right across distinct names, which is the case ADR-0044 reasoned about. The open
question is only what a repeated name should mean. Two points bear on it. The usual convention in
faceted search is to AND across fields and OR within a single field. And ADR-0044's escape hatch,
"issue two requests and union them", is least workable exactly here, since the caller must merge
two paged result sets client-side.

The API docs are inconsistent in the meantime, deliberately. The three collection listing
docstrings in app/api/v1/collection_router.py were written after this was found, so they use a
different name in the repeat example (?annotation=disruption&annotation=elm) and state
plainly that repeating one name is not useful. The four older ones, three in
dataset_router.py and one in shot_router.py, still put elm and elm:type-I side by side
and then say "repeat the parameter", which invites exactly the no-op above.

Those four were left alone on purpose: whatever this issue decides changes what they should
say, and rewording them twice is worse than once. They are the documentation half of the
acceptance criterion below. These docstrings surface in the OpenAPI schema, so they are read
by API users rather than only by maintainers.

Options, none chosen. Each amends ADR-0044 rather than filling a gap in it:

  1. AND across distinct names, OR within a repeated name. Follows the faceted-search convention.
    Changes the meaning of an endpoint that already ships, and makes the presence-plus-equality case
    widen the result rather than narrow it.
  2. Reject presence and equality on the same name with a 422, on the grounds that the request is
    ambiguous. Composes with any of the others rather than competing with them.
  3. Keep AND everywhere and document it plainly. Cheapest, but leaves the type-I/type-III query
    returning near-empty results with no hint as to why.
  4. Give OR its own syntax (elm:type-I|type-III, say) and leave repetition meaning AND. Keeps
    current behaviour and makes both intents expressible, at the cost of more grammar to specify.

Note also that test_repeated_annotation_param_parses_as_a_list covers two different names only, so
the same-name path currently has no test coverage either way.

Acceptance criteria

  • ADR recording the filter syntax and its scope boundary (what is deliberately not searchable).
  • Decide whether ADR-0044's AND rule needs a same-name exception (see the open question above), and cover the same-name case with tests.
  • Reword the four annotation docstrings that predate the finding (3 in dataset_router.py, 1 in shot_router.py) to match whatever is decided, so all seven agree.
  • Access-control predicate moved into the query, so pagination and result counts are correct.
  • Annotation presence and equality filters on the Shot and Dataset list endpoints.
  • Scalar-column filters on the Dataset list endpoints.
  • GET /devices/mast/shots?<annotation filter> returns shot 30421 and nothing else, as an integration
    test against the seeded demo.
  • The two-hop query answerable in one request.
  • Demo seed carries an elm annotation on a MAST-U shot, so the second example is demonstrable.

Related

  • #43 Adopt PostgreSQL for production persistence (ADR-0033). Blocks the indexed form.
  • internal #18 Time-bounded events on Shot/Dataset (ADR-0035). Model side, now landed on
    feat/feature-annotation. Note that internal #18's body still describes the rejected TimeInterval/events
    design and needs rewriting against the ADR as merged.
  • internal #19 Versioned reference geometry (ADR-0036). Its deferred role/validity-window filter is this
    same capability over applies_to.
  • #40 Schema-driven JSON-LD projection. Unrelated to search; was the mistaken referent.
  • internal #34 Device dataset listing scope. Blocks removing the ?device= workaround on /datasets;
    once it lands, the two-hop query moves to
    /devices/{d}/datasets?name=...&shot_annotation=....

Migrated from the internal tracker, where it was #32, opened 2026-08-05.

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start with ADR-0044 and the filter deferrals in ADR-0035, ADR-0036 and ADR-0043, then inspect the collection, dataset and shot router docstrings and shot_service.py:223. Review test_repeated_annotation_param_parses_as_a_list and the seeded-demo integration-test requirement before choosing the syntax and access-control approach. Done means the ADR, tests, documentation, query filtering, pagination, and two-hop example all meet the listed acceptance criteria.

Written by the indexing model from the issue text.

Assessment

Tech stack
fastapi, postgresql, python, sqlite
Domain
api, backend, databases, search, testing
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Active
Clarity
Mostly clear
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.