From ff4564c034fcd2fa7c98ac89444816f2a7596add Mon Sep 17 00:00:00 2001 From: Michael Barlow Date: Wed, 7 Oct 2026 15:12:34 -0600 Subject: [PATCH 1/4] feat(api): cascade filter options by the current selections - filter-options now accepts the list query's filter params and returns, per field, the values still reachable under every filter except that field's own, so multi-select keeps offering the un-ticked values - the six per-field branches share the list query's pivoted base and union into one statement, so the endpoint stays a single round trip - repointed resources no longer contribute dead option values; the shared base excludes them, which the previous standalone query did not - sorting moves to Python because SQLite and Postgres collate differently --- .../api/db/og_field_resource_actions.py | 14 +- deployments/api/src/stitch/api/db/queries.py | 134 +++++---- .../src/stitch/api/routers/oil_gas_fields.py | 3 + .../api/tests/db/test_resource_actions.py | 283 +++++++++++++++++- .../observability/test_query_name_actions.py | 3 +- .../api/tests/routers/test_resources_unit.py | 60 ++++ 6 files changed, 420 insertions(+), 77 deletions(-) diff --git a/deployments/api/src/stitch/api/db/og_field_resource_actions.py b/deployments/api/src/stitch/api/db/og_field_resource_actions.py index 7b4dda3a..4ba25aff 100644 --- a/deployments/api/src/stitch/api/db/og_field_resource_actions.py +++ b/deployments/api/src/stitch/api/db/og_field_resource_actions.py @@ -16,6 +16,7 @@ from stitch.api.observability.context import named_query from stitch.api.entities import ( FILTER_OPTION_FIELDS, + OGFieldFilterParams, OGFieldQueryParams, ) from stitch.api.db.og_field_source_actions import ( @@ -93,16 +94,23 @@ async def query( async def filter_options( session: AsyncSession, + params: OGFieldFilterParams, licensed_sources: Collection[OGSISrcKey] | None = None, ) -> dict[str, list[str]]: - """Distinct coalesced values for every filterable field, in one query.""" + """Each filterable field's values still reachable under ``params``. + + Still one query: the per-field branches are unioned in SQL. Each field is + narrowed by every *other* filter but not by its own, so ticking one value + leaves that dropdown's remaining values on offer. Sorting happens here + rather than in SQL because SQLite and Postgres order strings differently. + """ options: dict[str, list[str]] = {field: [] for field in FILTER_OPTION_FIELDS} with named_query("resources.filter_options"): for colname, value in await session.execute( - filter_option_rows(licensed_sources) + filter_option_rows(params, licensed_sources) ): options[colname].append(value) - return options + return {field: sorted(values) for field, values in options.items()} async def get( diff --git a/deployments/api/src/stitch/api/db/queries.py b/deployments/api/src/stitch/api/db/queries.py index 02ba5d5c..11c8729e 100644 --- a/deployments/api/src/stitch/api/db/queries.py +++ b/deployments/api/src/stitch/api/db/queries.py @@ -18,14 +18,17 @@ from sqlalchemy import ( CTE, ColumnElement, + CompoundSelect, Select, and_, asc, case, desc, func, + literal, or_, select, + union_all, ) from stitch.api.db.model import ( @@ -38,7 +41,11 @@ ResourceModel, ) from stitch.api.db.model.oil_gas_field_source_value import value_attr_for -from stitch.api.entities import FILTER_OPTION_FIELDS, OGFieldQueryParams +from stitch.api.entities import ( + FILTER_OPTION_FIELDS, + OGFieldFilterParams, + OGFieldQueryParams, +) from stitch.ogsi.model.types import OGSISrcKey # Single source of truth for the source-list field metadata. This is a shared @@ -66,7 +73,7 @@ _HEADER_SORT_FIELDS: Final[frozenset[str]] = frozenset({"id", "source", "resource_id"}) -def _filter_values(params: OGFieldQueryParams, field_name: str) -> list[Any]: +def _filter_values(params: OGFieldFilterParams, field_name: str) -> list[Any]: """The values an exact-match filter is set to, as a list. Multi-select fields arrive as lists; the rest are scalars, returned as a @@ -264,44 +271,35 @@ def coalesced_winner_rows( def filter_option_rows( + params: OGFieldFilterParams, licensed_sources: Collection[OGSISrcKey] | None = None, -) -> Select[tuple[str, str]]: - """Distinct winning ``(colname, value)`` pairs for every filterable field.""" - m = MembershipModel - v = OilGasFieldSourceValueModel - p = OGFieldSourcePriority - o = OGFieldResourceSourcePriority - - base = ( - select( - m.resource_id.label("resource_id"), - m.source.label("source"), - m.source_pk.label("source_pk"), - o.priority.label("override_priority"), - p.priority.label("default_priority"), - v.colname.label("colname"), - v.value_text, - ) - .select_from(m) - .join(v, v.source_pk == m.source_pk) - .join(p, p.source == m.source) - .outerjoin(o, _override_join(m.resource_id)) - .where( - m.status == MembershipStatus.ACTIVE, - v.colname.in_(FILTER_OPTION_FIELDS), - ) - ) - if licensed_sources is not None: - base = base.where(m.source.in_(list(dict.fromkeys(licensed_sources)))) - - ranked = add_ranking(base.cte("filter_option_base")).cte("filter_option_ranked") - c = ranked.c - return ( - select(c.colname, c.value_text) - .where(c.value_text.is_not(None)) # rn == 1 already applied by add_ranking - .distinct() - .order_by(c.colname, c.value_text) - ) +) -> CompoundSelect: + """Distinct ``(colname, value)`` pairs still reachable under ``params``. + + One branch per filterable field over the shared pivoted base, unioned into a + single statement so the endpoint stays one round trip. Each branch applies + every filter *except* the branch field's own, so a field is never narrowed by + its own selection: after ticking one country the country list still offers + the rest. Clearing the field on a copy of ``params`` is what expresses that, + which keeps ``q``, multi-select, and the non-faceted exact filters working + here exactly as they do on the list query. + + Unordered on purpose: SQLite and Postgres disagree on string collation, so + the caller sorts in Python. + """ + base = _pivoted_resource_base(EXACT_MATCH_FIELDS, licensed_sources) + branches: list[Select[tuple[str, str]]] = [] + for field_name in FILTER_OPTION_FIELDS: + column = _require_column(base, field_name) + branch = select( + literal(field_name).label("colname"), column.label("value") + ).where(column.is_not(None)) + for cond in _build_field_conditions( + base, params.model_copy(update={field_name: None}) + ): + branch = branch.where(cond) + branches.append(branch.distinct()) + return union_all(*branches) def field_source_candidates( @@ -404,40 +402,50 @@ def _resource_universe() -> Select[tuple[int]]: ) +def _pivoted_resource_base( + involved: Collection[str], + licensed_sources: Collection[OGSISrcKey] | None = None, +) -> CTE: + """One row per listable resource, with ``involved`` pivoted to wide columns. + + The coalesced winner per ``(resource, colname)`` pivoted into one named + column per field, LEFT JOINed onto the membership universe so a resource + with no licensed values keeps its row (null-shell) and is dropped only by an + actual field filter. Shared by the list query and the filter-option query so + both see the same rows under the same filters. + """ + universe = _resource_universe().cte("resource_universe") + base_cte = construct_base_query_statement(licensed_sources) + ranked = add_ranking(base_cte).cte("ranked") + pivot = _add_pivot_columns( + select(ranked.c.resource_id.label("resource_id")), + involved, + ranked.c.colname, + lambda field_name: getattr(ranked.c, value_attr_for(field_name)), + ) + pivot_cte = pivot.group_by(ranked.c.resource_id).cte("resource_pivot") + + resource_base = select(universe.c.resource_id) + for field_name in involved: + resource_base = resource_base.add_columns(pivot_cte.c[field_name]) + return resource_base.select_from( + universe.outerjoin(pivot_cte, pivot_cte.c.resource_id == universe.c.resource_id) + ).cte("resource_base") + + def base_resource_query( params: OGFieldQueryParams, licensed_sources: Collection[OGSISrcKey] | None = None, ) -> Select[tuple[int]]: involved = _participating_columns(params) - universe = _resource_universe().cte("resource_universe") if not involved: # No value field filtered or sorted -> the universe alone (every active # resource); only the id filter below can narrow it. - base = universe + base = _resource_universe().cte("resource_universe") conditions: list[ColumnElement[bool]] = [] else: - base_cte = construct_base_query_statement(licensed_sources) - ranked = add_ranking(base_cte).cte("ranked") - pivot = _add_pivot_columns( - select(ranked.c.resource_id.label("resource_id")), - involved, - ranked.c.colname, - lambda field_name: getattr(ranked.c, value_attr_for(field_name)), - ) - pivot_cte = pivot.group_by(ranked.c.resource_id).cte("resource_pivot") - - # LEFT JOIN the licensed/coalesced pivot onto the membership universe: a - # resource with no licensed values keeps its row (null-shell) but is - # dropped by any field filter below. - resource_base = select(universe.c.resource_id) - for field_name in involved: - resource_base = resource_base.add_columns(pivot_cte.c[field_name]) - base = resource_base.select_from( - universe.outerjoin( - pivot_cte, pivot_cte.c.resource_id == universe.c.resource_id - ) - ).cte("resource_base") + base = _pivoted_resource_base(involved, licensed_sources) conditions = _build_field_conditions(base, params) stmt = select(base.c.resource_id) @@ -489,7 +497,7 @@ def _require_column(cte: CTE | Select, field_name: str) -> ColumnElement[Any]: def _build_field_conditions( cte: CTE | Select, - params: OGFieldQueryParams, + params: OGFieldFilterParams, ) -> list[ColumnElement[bool]]: """Path-agnostic q-ILIKE + exact-match filters over the pivoted columns. diff --git a/deployments/api/src/stitch/api/routers/oil_gas_fields.py b/deployments/api/src/stitch/api/routers/oil_gas_fields.py index 9e0c5c04..0f8d9c25 100644 --- a/deployments/api/src/stitch/api/routers/oil_gas_fields.py +++ b/deployments/api/src/stitch/api/routers/oil_gas_fields.py @@ -18,6 +18,7 @@ MergeCandidateDetailView, MergeCandidateReviewRequest, MergeCandidateView, + OGFieldFilterParams, OGFieldQueryParams, PaginatedResponse, SetFieldPriorityRequest, @@ -91,9 +92,11 @@ async def get_resource_filter_options( uow: UnitOfWorkDep, _user: CurrentUser, claims: Claims, + params: Annotated[OGFieldFilterParams, Query()], ) -> OGFieldFilterOptionsResponse: opts = await resource_actions.filter_options( session=uow.session, + params=params, licensed_sources=licensed_sources(claims), ) return OGFieldFilterOptionsResponse(**opts) diff --git a/deployments/api/tests/db/test_resource_actions.py b/deployments/api/tests/db/test_resource_actions.py index 01ea3dad..94149673 100644 --- a/deployments/api/tests/db/test_resource_actions.py +++ b/deployments/api/tests/db/test_resource_actions.py @@ -24,6 +24,7 @@ from stitch.api.db.queries import filter_option_rows from stitch.api.entities import ( FILTER_OPTION_FIELDS, + OGFieldFilterParams, OGFieldQueryParams, User, ) @@ -1016,7 +1017,9 @@ async def test_returns_distinct_sorted_coalesced_values( {"source": "rmi", "country": None}, ) - options = await resource_actions.filter_options(seeded_integration_session) + options = await resource_actions.filter_options( + seeded_integration_session, OGFieldFilterParams() + ) assert options["country"] == ["CAN", "USA"] @@ -1040,6 +1043,7 @@ async def test_honors_licensed_sources_after_coalescing( options = await resource_actions.filter_options( seeded_integration_session, + OGFieldFilterParams(), licensed_sources=frozenset({"gem", "wm", "llm"}), ) @@ -1063,12 +1067,14 @@ async def test_licensing_promotes_next_priority_value( with_wm = await resource_actions.filter_options( seeded_integration_session, + OGFieldFilterParams(), licensed_sources=frozenset({"gem", "wm"}), ) assert with_wm["country"] == ["USA"] without_wm = await resource_actions.filter_options( seeded_integration_session, + OGFieldFilterParams(), licensed_sources=frozenset({"gem"}), ) assert without_wm["country"] == ["CAN"] @@ -1097,7 +1103,9 @@ async def test_excludes_inactive_memberships( inactive_membership.status = MembershipStatus.INACTIVE await seeded_integration_session.flush() - options = await resource_actions.filter_options(seeded_integration_session) + options = await resource_actions.filter_options( + seeded_integration_session, OGFieldFilterParams() + ) assert options["country"] == ["USA"] @@ -1113,7 +1121,9 @@ async def test_returns_every_field_including_empty_ones( {"source": "rmi", "country": "USA"}, ) - options = await resource_actions.filter_options(seeded_integration_session) + options = await resource_actions.filter_options( + seeded_integration_session, OGFieldFilterParams() + ) assert set(options.keys()) == set(FILTER_OPTION_FIELDS) assert options["country"] == ["USA"] @@ -1148,16 +1158,24 @@ async def test_returns_each_field_from_one_pass( }, ) - options = await resource_actions.filter_options(seeded_integration_session) + options = await resource_actions.filter_options( + seeded_integration_session, OGFieldFilterParams() + ) assert options["country"] == ["CAN", "USA"] assert options["state_province"] == ["Alberta", "Texas"] assert options["basin"] == ["Permian", "Texas"] assert options["region"] == ["North America"] - def test_postgres_distinct_query_orders_by_selected_columns(self): - """``filter_option_rows`` compiles on Postgres.""" + def test_postgres_query_unions_one_distinct_branch_per_field(self): + """``filter_option_rows`` compiles on Postgres as one unioned statement. + + One ``DISTINCT`` branch per filterable field, unioned, so the endpoint + stays a single round trip. Sorting is deliberately absent: the two + dialects collate strings differently, so the action sorts in Python. + """ stmt = filter_option_rows( + OGFieldFilterParams(), licensed_sources=frozenset({"gem", "wm", "rmi", "llm"}), ) @@ -1168,10 +1186,27 @@ def test_postgres_distinct_query_orders_by_selected_columns(self): ) ) - assert "SELECT DISTINCT" in sql - assert "ORDER BY" in sql - ordered = sql[sql.index("ORDER BY") :] - assert "colname" in ordered and "value_text" in ordered + assert sql.count("SELECT DISTINCT") >= len(FILTER_OPTION_FIELDS) + assert sql.count("UNION ALL") == len(FILTER_OPTION_FIELDS) - 1 + for field in FILTER_OPTION_FIELDS: + assert f"'{field}' AS colname" in sql + + def test_self_exclusion_omits_only_the_branch_fields_own_filter(self): + """A field's branch drops its own filter and keeps every other one.""" + stmt = filter_option_rows( + OGFieldFilterParams(country=["CAN"], state_province=["Alberta"]), + ) + + sql = str( + stmt.compile( + dialect=postgresql.dialect(), + compile_kwargs={"literal_binds": True}, + ) + ) + + # Applied in every branch but the field's own -> one fewer than six. + assert sql.count("IN ('CAN')") == len(FILTER_OPTION_FIELDS) - 1 + assert sql.count("IN ('Alberta')") == len(FILTER_OPTION_FIELDS) - 1 @pytest.mark.anyio async def test_only_unlicensed_selected_sources_still_return_resource( @@ -1340,6 +1375,234 @@ async def test_repointed_resources_are_excluded( assert [item.id for item in items] == [root_id] +class TestResourceFilterOptionsCascade: + """Options offered for one field reflect the *other* fields' selections. + + The bug this closes: with global option lists a user could pick Country = + Canada, then pick Alaska from a State/Province list that still offered every + state, and land on "No resources match". + """ + + async def _seed_two_countries( + self, + session: AsyncSession, + user: User, + ) -> None: + await _create_resource_with_sources( + session, + user, + { + "source": "gem", + "name": "Canadian field", + "country": "CAN", + "state_province": "Alberta", + "basin": "Western Canadian", + }, + ) + await _create_resource_with_sources( + session, + user, + { + "source": "gem", + "name": "American field", + "country": "USA", + "state_province": "Alaska", + "basin": "North Slope", + }, + ) + + @pytest.mark.anyio + async def test_country_narrows_state_province( + self, + seeded_integration_session: AsyncSession, + test_user: User, + ): + await self._seed_two_countries(seeded_integration_session, test_user) + + options = await resource_actions.filter_options( + seeded_integration_session, OGFieldFilterParams(country=["CAN"]) + ) + + assert options["state_province"] == ["Alberta"] + assert options["basin"] == ["Western Canadian"] + + @pytest.mark.anyio + async def test_state_province_narrows_country( + self, + seeded_integration_session: AsyncSession, + test_user: User, + ): + """The reverse direction: picking a province narrows Country.""" + await self._seed_two_countries(seeded_integration_session, test_user) + + options = await resource_actions.filter_options( + seeded_integration_session, + OGFieldFilterParams(state_province=["Alberta"]), + ) + + assert options["country"] == ["CAN"] + + @pytest.mark.anyio + async def test_basin_spanning_two_countries_keeps_both( + self, + seeded_integration_session: AsyncSession, + test_user: User, + ): + """Narrowing is by reachability, not by a one-to-one hierarchy.""" + await _create_resource_with_sources( + seeded_integration_session, + test_user, + {"source": "gem", "country": "CAN", "basin": "Williston"}, + ) + await _create_resource_with_sources( + seeded_integration_session, + test_user, + {"source": "gem", "country": "USA", "basin": "Williston"}, + ) + await _create_resource_with_sources( + seeded_integration_session, + test_user, + {"source": "gem", "country": "NOR", "basin": "North Sea"}, + ) + + options = await resource_actions.filter_options( + seeded_integration_session, OGFieldFilterParams(basin=["Williston"]) + ) + + assert options["country"] == ["CAN", "USA"] + + @pytest.mark.anyio + async def test_field_is_not_narrowed_by_its_own_selection( + self, + seeded_integration_session: AsyncSession, + test_user: User, + ): + """Multi-select keeps working: Country still offers the un-ticked ones.""" + await self._seed_two_countries(seeded_integration_session, test_user) + + options = await resource_actions.filter_options( + seeded_integration_session, OGFieldFilterParams(country=["CAN"]) + ) + + assert options["country"] == ["CAN", "USA"] + + @pytest.mark.anyio + async def test_multi_select_unions_the_other_fields( + self, + seeded_integration_session: AsyncSession, + test_user: User, + ): + """Two countries ticked -> the other fields offer both countries' values.""" + await self._seed_two_countries(seeded_integration_session, test_user) + + options = await resource_actions.filter_options( + seeded_integration_session, + OGFieldFilterParams(country=["CAN", "USA"]), + ) + + assert options["state_province"] == ["Alaska", "Alberta"] + + @pytest.mark.anyio + async def test_search_term_cascades_into_every_field( + self, + seeded_integration_session: AsyncSession, + test_user: User, + ): + await self._seed_two_countries(seeded_integration_session, test_user) + + options = await resource_actions.filter_options( + seeded_integration_session, OGFieldFilterParams(q="Canadian") + ) + + assert options["country"] == ["CAN"] + assert options["state_province"] == ["Alberta"] + + @pytest.mark.anyio + async def test_search_term_is_not_self_excluded( + self, + seeded_integration_session: AsyncSession, + test_user: User, + ): + """``q`` is never a branch target, so it narrows even the fields it + matches on. ``basin`` is searchable, so a term matching only the Canadian + basin must still leave Basin showing that one value alone.""" + await self._seed_two_countries(seeded_integration_session, test_user) + + options = await resource_actions.filter_options( + seeded_integration_session, OGFieldFilterParams(q="Western Canadian") + ) + + assert options["basin"] == ["Western Canadian"] + + @pytest.mark.anyio + async def test_unfiltered_params_offer_every_value( + self, + seeded_integration_session: AsyncSession, + test_user: User, + ): + """No filters set -> the pre-cascade behaviour, every reachable value.""" + await self._seed_two_countries(seeded_integration_session, test_user) + + options = await resource_actions.filter_options( + seeded_integration_session, OGFieldFilterParams() + ) + + assert options["country"] == ["CAN", "USA"] + assert options["state_province"] == ["Alaska", "Alberta"] + + @pytest.mark.anyio + async def test_excludes_repointed_resources( + self, + seeded_integration_session: AsyncSession, + test_user: User, + ): + """A merged-away resource's values are not offered. + + Behaviour change: the previous query never joined ``ResourceModel``, so a + repointed resource's values stayed in the dropdowns while matching no + list row. Sharing the list query's base removes them. + """ + root_id = await _create_resource_with_sources( + seeded_integration_session, + test_user, + {"source": "gem", "country": "USA"}, + ) + await _create_resource_with_sources( + seeded_integration_session, + test_user, + {"source": "gem", "country": "CAN"}, + repointed_to=root_id, + ) + + options = await resource_actions.filter_options( + seeded_integration_session, OGFieldFilterParams() + ) + + assert options["country"] == ["USA"] + + @pytest.mark.anyio + async def test_cascade_respects_licensed_sources( + self, + seeded_integration_session: AsyncSession, + test_user: User, + ): + """Filtering on an unlicensed source's value offers nothing elsewhere.""" + await _create_resource_with_sources( + seeded_integration_session, + test_user, + {"source": "wm", "country": "USA", "basin": "Permian"}, + ) + + options = await resource_actions.filter_options( + seeded_integration_session, + OGFieldFilterParams(country=["USA"]), + licensed_sources=frozenset({"gem"}), + ) + + assert options["basin"] == [] + assert options["country"] == [] + + class TestResourcePriorityOverride: """A per-field override re-ranks sources, flipping the coalesced winner.""" diff --git a/deployments/api/tests/observability/test_query_name_actions.py b/deployments/api/tests/observability/test_query_name_actions.py index 619d2af3..b2c542cc 100644 --- a/deployments/api/tests/observability/test_query_name_actions.py +++ b/deployments/api/tests/observability/test_query_name_actions.py @@ -28,6 +28,7 @@ from stitch.api.entities import ( MergeCandidateCreateRequest, MergeCandidateReviewRequest, + OGFieldFilterParams, OGFieldQueryParams, User, ) @@ -172,7 +173,7 @@ async def test_resource_read_labels( ) captured_query_events.clear() - await resource_actions.filter_options(session) + await resource_actions.filter_options(session, OGFieldFilterParams()) _assert_labels(captured_query_events, {"resources.filter_options"}) captured_query_events.clear() diff --git a/deployments/api/tests/routers/test_resources_unit.py b/deployments/api/tests/routers/test_resources_unit.py index 5585acdc..5efc4941 100644 --- a/deployments/api/tests/routers/test_resources_unit.py +++ b/deployments/api/tests/routers/test_resources_unit.py @@ -324,3 +324,63 @@ async def override_get_uow(): assert call_kwargs["licensed_sources"] == frozenset( {"rmi", "gem", "wm", "ccr", "alb", "bc", "nor", "llm"} ) + + @pytest.mark.anyio + async def test_passes_filter_params(self, async_client, mock_uow): + """The current selections reach the action, so options can cascade.""" + + async def override_get_uow(): + yield mock_uow + + app.dependency_overrides[get_uow] = override_get_uow + + with patch("stitch.api.routers.oil_gas_fields.resource_actions") as mock_repo: + mock_repo.filter_options = AsyncMock( + return_value=dict( + basin=[], + country=[], + field_status=[], + primary_hydrocarbon_group=[], + region=[], + state_province=[], + ) + ) + + response = await async_client.get( + "/oil-gas-fields/filter-options" + "?country=NOR&country=SAU&state_province=Alberta&q=ghawar" + ) + + assert response.status_code == 200 + params = mock_repo.filter_options.call_args.kwargs["params"] + assert params.country == ["NOR", "SAU"] + assert params.state_province == ["Alberta"] + assert params.q == "ghawar" + + @pytest.mark.anyio + async def test_no_query_string_still_works(self, async_client, mock_uow): + """Every param is optional: callers that send none are unaffected.""" + + async def override_get_uow(): + yield mock_uow + + app.dependency_overrides[get_uow] = override_get_uow + + with patch("stitch.api.routers.oil_gas_fields.resource_actions") as mock_repo: + mock_repo.filter_options = AsyncMock( + return_value=dict( + basin=[], + country=[], + field_status=[], + primary_hydrocarbon_group=[], + region=[], + state_province=[], + ) + ) + + response = await async_client.get("/oil-gas-fields/filter-options") + + assert response.status_code == 200 + params = mock_repo.filter_options.call_args.kwargs["params"] + assert params.country is None + assert params.q is None From 62c6a6d84099b5eb211b787cb07719664c158f68 Mon Sep 17 00:00:00 2001 From: Michael Barlow Date: Wed, 7 Oct 2026 15:12:57 -0600 Subject: [PATCH 2/4] feat(frontend): send the active filters with the filter-options request - getResourceFilterOptions takes filters and q, serialized by the same helper getResources uses so both requests agree on the wire format - the react-query key carries the selections, so each combination caches separately; with no filters set the URL is unchanged - the mock path applies the same leave-one-out rule as the backend --- .../stitch-frontend/src/hooks/useResources.js | 28 ++++++++++----- .../stitch-frontend/src/queries/api.js | 35 ++++++++++++++----- .../stitch-frontend/src/queries/api.test.js | 18 ++++++++++ .../stitch-frontend/src/queries/resources.js | 12 ++++--- 4 files changed, 72 insertions(+), 21 deletions(-) diff --git a/deployments/stitch-frontend/src/hooks/useResources.js b/deployments/stitch-frontend/src/hooks/useResources.js index 14009924..dc5c572d 100644 --- a/deployments/stitch-frontend/src/hooks/useResources.js +++ b/deployments/stitch-frontend/src/hooks/useResources.js @@ -55,10 +55,13 @@ function useResourcesReal( }); } -function useResourceFilterOptionsReal(endpoint = "resources", enabled = true) { +function useResourceFilterOptionsReal( + endpoint = "resources", + { filters = {}, q, enabled = true } = {}, +) { const config = useConfig(); return useAuthenticatedQuery({ - ...resourceQueries.filterOptions(config, endpoint), + ...resourceQueries.filterOptions(config, endpoint, filters, q), enabled, }); } @@ -214,15 +217,19 @@ function getMockResourcePage({ }; } -function getMockFilterOptions() { +function getMockFilterOptions({ filters = {}, q } = {}) { + const searched = applyMockSearch(MOCK_RESOURCE_ITEMS, q); + return Object.fromEntries( FILTER_FIELDS.map(({ key: field }) => [ field, Array.from( new Set( - MOCK_RESOURCE_ITEMS.map((resource) => - getResourceField(resource, field), - ) + // Leave-one-out, matching the backend: a field's options are the + // values reachable under every *other* filter, so ticking one value + // never hides that field's remaining values. + applyMockFilters(searched, { ...filters, [field]: [] }) + .map((resource) => getResourceField(resource, field)) .filter((value) => value != null && value !== "") .map(String), ), @@ -269,10 +276,13 @@ function useResourcesMock( }); } -function useResourceFilterOptionsMock(endpoint = "resources", enabled = true) { +function useResourceFilterOptionsMock( + endpoint = "resources", + { filters = {}, q, enabled = true } = {}, +) { return useQuery({ - ...resourceQueries.filterOptions(UNUSED_MOCK_CONFIG, endpoint), - queryFn: () => Promise.resolve(getMockFilterOptions()), + ...resourceQueries.filterOptions(UNUSED_MOCK_CONFIG, endpoint, filters, q), + queryFn: () => Promise.resolve(getMockFilterOptions({ filters, q })), enabled, }); } diff --git a/deployments/stitch-frontend/src/queries/api.js b/deployments/stitch-frontend/src/queries/api.js index fa1cfdb9..f45d4521 100644 --- a/deployments/stitch-frontend/src/queries/api.js +++ b/deployments/stitch-frontend/src/queries/api.js @@ -1,16 +1,27 @@ -export async function getResources( - config, - fetcher, - endpoint = "resources", - { page = 1, page_size = 50, filters = {}, q, sort_by, sort_order } = {}, -) { - const params = new URLSearchParams({ page, page_size }); +// The list query and the filter-options query must serialize the current +// selections identically: the options a dropdown offers are the values still +// reachable under exactly the filters the list is about to apply. +function appendFilterParams(params, filters = {}, q) { for (const [key, values] of Object.entries(filters)) { for (const v of values) { params.append(key, v); } } if (q) params.set("q", q); + return params; +} + +export async function getResources( + config, + fetcher, + endpoint = "resources", + { page = 1, page_size = 50, filters = {}, q, sort_by, sort_order } = {}, +) { + const params = appendFilterParams( + new URLSearchParams({ page, page_size }), + filters, + q, + ); if (sort_by) params.set("sort_by", sort_by); if (sort_order) params.set("sort_order", sort_order); const url = `${config.apiBaseUrl}/${endpoint}/?${params}`; @@ -27,8 +38,16 @@ export async function getResourceFilterOptions( config, fetcher, endpoint = "resources", + { filters = {}, q } = {}, ) { - const url = `${config.apiBaseUrl}/${endpoint}/filter-options`; + const query = appendFilterParams( + new URLSearchParams(), + filters, + q, + ).toString(); + const url = `${config.apiBaseUrl}/${endpoint}/filter-options${ + query ? `?${query}` : "" + }`; const response = await fetcher(url); if (!response.ok) { const error = new Error(`HTTP error! status: ${response.status}`); diff --git a/deployments/stitch-frontend/src/queries/api.test.js b/deployments/stitch-frontend/src/queries/api.test.js index 5722bb08..b8a127e4 100644 --- a/deployments/stitch-frontend/src/queries/api.test.js +++ b/deployments/stitch-frontend/src/queries/api.test.js @@ -156,6 +156,24 @@ describe("API Functions", () => { expect(result).toEqual(mockOptions); }); + it("sends the current filters and search so the options cascade", async () => { + mockFetcher.mockResolvedValueOnce({ + ok: true, + status: 200, + json: async () => ({}), + }); + + await getResourceFilterOptions(config, mockFetcher, "oil-gas-fields", { + filters: { country: ["NOR", "SAU"], basin: [] }, + q: "ghawar", + }); + + expect(mockFetcher).toHaveBeenCalledWith( + "http://localhost:8000/api/v1/oil-gas-fields/filter-options" + + "?country=NOR&country=SAU&q=ghawar", + ); + }); + it("throws error when filter options response is not ok", async () => { mockFetcher.mockResolvedValueOnce({ ok: false, diff --git a/deployments/stitch-frontend/src/queries/resources.js b/deployments/stitch-frontend/src/queries/resources.js index 4e8332eb..3559c9d3 100644 --- a/deployments/stitch-frontend/src/queries/resources.js +++ b/deployments/stitch-frontend/src/queries/resources.js @@ -28,9 +28,10 @@ const keys = { all: (endpoint = "resources") => [endpoint], lists: (endpoint = "resources") => [...keys.all(endpoint), "list"], list: (endpoint = "resources", filters) => [...keys.lists(endpoint), filters], - filterOptions: (endpoint = "resources") => [ + filterOptions: (endpoint = "resources", params) => [ ...keys.all(endpoint), "filter-options", + params, ], details: (endpoint = "resources") => [...keys.all(endpoint), "detail"], detail: (endpoint = "resources", id) => [...keys.details(endpoint), id], @@ -92,10 +93,13 @@ export const resourceQueries = { placeholderData: keepPreviousData, }), - filterOptions: (config, endpoint = "resources") => + // The options are per-selection, so the selections belong in the key, in + // the same shape the list key uses. + filterOptions: (config, endpoint = "resources", filters = {}, q) => queryOptions({ - queryKey: keys.filterOptions(endpoint), - queryFn: (fetcher) => getResourceFilterOptions(config, fetcher, endpoint), + queryKey: keys.filterOptions(endpoint, { ...filters, q }), + queryFn: (fetcher) => + getResourceFilterOptions(config, fetcher, endpoint, { filters, q }), staleTime: DEFAULT_STALE_TIME, }), From 0e727f4bed76355a76e81725e0167bc803455573 Mon Sep 17 00:00:00 2001 From: Michael Barlow Date: Wed, 7 Oct 2026 15:14:59 -0600 Subject: [PATCH 3/4] feat(frontend): drive the filter dropdowns from the settled selections - FilterBar takes the options-query params as a prop; ResourcesView passes the debounced list params, so one settled change costs one options request rather than one per click - the ticked checkboxes still read the live filters prop, so they keep responding immediately --- .../src/components/FilterBar.jsx | 16 ++++++++++++-- .../src/components/ResourcesView.jsx | 4 ++++ .../src/components/ResourcesView.test.jsx | 22 ++++++++++++++++++- 3 files changed, 39 insertions(+), 3 deletions(-) diff --git a/deployments/stitch-frontend/src/components/FilterBar.jsx b/deployments/stitch-frontend/src/components/FilterBar.jsx index bad32b52..7a82d7d8 100644 --- a/deployments/stitch-frontend/src/components/FilterBar.jsx +++ b/deployments/stitch-frontend/src/components/FilterBar.jsx @@ -2,8 +2,20 @@ import FilterDropdown from "./FilterDropdown"; import { FILTER_FIELDS, EMPTY_FILTERS } from "../config/filters"; import { useResourceFilterOptions } from "../hooks/useResources"; -export default function FilterBar({ endpoint, filters, onFiltersChange }) { - const { data: filterOptions } = useResourceFilterOptions(endpoint); +export default function FilterBar({ + endpoint, + filters, + onFiltersChange, + optionParams, +}) { + // `filters` drives the ticked boxes and updates on every click. The options + // request runs on `optionParams`, which the caller debounces alongside the + // list request, so a settled change costs one options request, not one per + // click. + const { data: filterOptions } = useResourceFilterOptions( + endpoint, + optionParams, + ); // Flatten active filters into chips: [{ field, label, value, displayValue }, ...] // `value` is the stored/API value; `displayValue` is what the user sees. diff --git a/deployments/stitch-frontend/src/components/ResourcesView.jsx b/deployments/stitch-frontend/src/components/ResourcesView.jsx index 023e3bad..0b2db1cf 100644 --- a/deployments/stitch-frontend/src/components/ResourcesView.jsx +++ b/deployments/stitch-frontend/src/components/ResourcesView.jsx @@ -191,6 +191,10 @@ export default function ResourcesView({ className = "", endpoint }) { endpoint={endpoint} filters={filters} onFiltersChange={handleFiltersChange} + optionParams={{ + filters: settledListParams.filters, + q: settledListParams.q, + }} /> diff --git a/deployments/stitch-frontend/src/components/ResourcesView.test.jsx b/deployments/stitch-frontend/src/components/ResourcesView.test.jsx index 142c2524..489c20a4 100644 --- a/deployments/stitch-frontend/src/components/ResourcesView.test.jsx +++ b/deployments/stitch-frontend/src/components/ResourcesView.test.jsx @@ -555,7 +555,27 @@ describe("ResourcesView", () => { renderWithQueryClient(); expect(useResourceFilterOptions).toHaveBeenCalledTimes(1); - expect(useResourceFilterOptions).toHaveBeenCalledWith(ENDPOINT); + expect(useResourceFilterOptions).toHaveBeenCalledWith( + ENDPOINT, + expect.objectContaining({ filters: expect.any(Object) }), + ); + }); + + it("sends the active filters and search to the options query", () => { + vi.mocked(useResources).mockReturnValue({ + ...defaultHookReturn, + data: mockResourceData, + }); + + renderWithQueryClient(, { + initialEntries: ["/?country=NOR&q=ghawar"], + }); + + expect(useResourceFilterOptions).toHaveBeenCalledTimes(1); + expect(useResourceFilterOptions).toHaveBeenLastCalledWith(ENDPOINT, { + filters: expect.objectContaining({ country: ["NOR"] }), + q: "ghawar", + }); }); it("shows country options as conventional names but filters by the code", () => { From 6c23979f623a05ba30e8515b57b7c9e86515f956 Mon Sep 17 00:00:00 2001 From: Michael Barlow Date: Wed, 7 Oct 2026 16:59:19 -0600 Subject: [PATCH 4/4] refactor(api): build the option cascade on its own query, not the list base - filter_option_rows goes back to the standalone base it had before, pivoting the ranked rows locally, so base_resource_query is byte-identical to its original form and the shared helper is gone - the colname narrowing is now dynamic: the faceted fields, plus only the attributes the current filters and search term actually read - no behaviour change; the unfiltered query returns the same rows and the regression against the previous implementation is gone --- deployments/api/src/stitch/api/db/queries.py | 130 +++++++++++++------ 1 file changed, 87 insertions(+), 43 deletions(-) diff --git a/deployments/api/src/stitch/api/db/queries.py b/deployments/api/src/stitch/api/db/queries.py index 11c8729e..e7e1c206 100644 --- a/deployments/api/src/stitch/api/db/queries.py +++ b/deployments/api/src/stitch/api/db/queries.py @@ -276,26 +276,80 @@ def filter_option_rows( ) -> CompoundSelect: """Distinct ``(colname, value)`` pairs still reachable under ``params``. - One branch per filterable field over the shared pivoted base, unioned into a - single statement so the endpoint stays one round trip. Each branch applies - every filter *except* the branch field's own, so a field is never narrowed by - its own selection: after ticking one country the country list still offers - the rest. Clearing the field on a copy of ``params`` is what expresses that, - which keeps ``q``, multi-select, and the non-faceted exact filters working - here exactly as they do on the list query. + One branch per filterable field, unioned into a single statement so the + endpoint stays one round trip. Each branch applies every filter *except* the + branch field's own, so a field is never narrowed by its own selection: after + ticking one country the country list still offers the rest. Clearing the + field on a copy of ``params`` is what expresses that, which keeps ``q``, + multi-select, and the non-faceted exact filters working here exactly as they + do on the list query. Unordered on purpose: SQLite and Postgres disagree on string collation, so the caller sorts in Python. """ - base = _pivoted_resource_base(EXACT_MATCH_FIELDS, licensed_sources) + m = MembershipModel + v = OilGasFieldSourceValueModel + r = ResourceModel + p = OGFieldSourcePriority + o = OGFieldResourceSourcePriority + + # The faceted fields are always needed, since each one is a branch target. + # The rest are needed only when a filter or the search term actually reads + # them, so ranking and pivoting stay as narrow as the current params allow. + involved = list(FILTER_OPTION_FIELDS) + for field_name in EXACT_MATCH_FIELDS: + if _filter_values(params, field_name): + involved.append(field_name) + if params.q: + involved += Q_FIELDS + involved = list(dict.fromkeys(involved)) + + base = ( + select( + m.resource_id.label("resource_id"), + m.source.label("source"), + m.source_pk.label("source_pk"), + o.priority.label("override_priority"), + p.priority.label("default_priority"), + v.colname.label("colname"), + v.value_text, + ) + .select_from(m) + .join(r, r.id == m.resource_id) + .join(v, v.source_pk == m.source_pk) + .join(p, p.source == m.source) + .outerjoin(o, _override_join(m.resource_id)) + .where( + # A merged-away resource matches no list row, so its values must not + # be offered as options either. + r.repointed_id.is_(None), + m.status == MembershipStatus.ACTIVE, + v.colname.in_(involved), + ) + ) + if licensed_sources is not None: + base = base.where(m.source.in_(list(dict.fromkeys(licensed_sources)))) + + ranked = add_ranking(base.cte("filter_option_base")).cte("filter_option_ranked") + + # Pivot the winning values to one row per resource: a branch listing one + # field's values has to filter on the *other* fields of the same resource. + pivot = _add_pivot_columns( + select(ranked.c.resource_id.label("resource_id")), + involved, + ranked.c.colname, + lambda field_name: getattr(ranked.c, value_attr_for(field_name)), + ) + pivot_cte = pivot.group_by(ranked.c.resource_id).cte("filter_option_pivot") + branches: list[Select[tuple[str, str]]] = [] for field_name in FILTER_OPTION_FIELDS: - column = _require_column(base, field_name) + column = _require_column(pivot_cte, field_name) branch = select( literal(field_name).label("colname"), column.label("value") ).where(column.is_not(None)) for cond in _build_field_conditions( - base, params.model_copy(update={field_name: None}) + pivot_cte, params.model_copy(update={field_name: None}) ): branch = branch.where(cond) branches.append(branch.distinct()) @@ -402,50 +456,40 @@ def _resource_universe() -> Select[tuple[int]]: ) -def _pivoted_resource_base( - involved: Collection[str], - licensed_sources: Collection[OGSISrcKey] | None = None, -) -> CTE: - """One row per listable resource, with ``involved`` pivoted to wide columns. - - The coalesced winner per ``(resource, colname)`` pivoted into one named - column per field, LEFT JOINed onto the membership universe so a resource - with no licensed values keeps its row (null-shell) and is dropped only by an - actual field filter. Shared by the list query and the filter-option query so - both see the same rows under the same filters. - """ - universe = _resource_universe().cte("resource_universe") - base_cte = construct_base_query_statement(licensed_sources) - ranked = add_ranking(base_cte).cte("ranked") - pivot = _add_pivot_columns( - select(ranked.c.resource_id.label("resource_id")), - involved, - ranked.c.colname, - lambda field_name: getattr(ranked.c, value_attr_for(field_name)), - ) - pivot_cte = pivot.group_by(ranked.c.resource_id).cte("resource_pivot") - - resource_base = select(universe.c.resource_id) - for field_name in involved: - resource_base = resource_base.add_columns(pivot_cte.c[field_name]) - return resource_base.select_from( - universe.outerjoin(pivot_cte, pivot_cte.c.resource_id == universe.c.resource_id) - ).cte("resource_base") - - def base_resource_query( params: OGFieldQueryParams, licensed_sources: Collection[OGSISrcKey] | None = None, ) -> Select[tuple[int]]: involved = _participating_columns(params) + universe = _resource_universe().cte("resource_universe") if not involved: # No value field filtered or sorted -> the universe alone (every active # resource); only the id filter below can narrow it. - base = _resource_universe().cte("resource_universe") + base = universe conditions: list[ColumnElement[bool]] = [] else: - base = _pivoted_resource_base(involved, licensed_sources) + base_cte = construct_base_query_statement(licensed_sources) + ranked = add_ranking(base_cte).cte("ranked") + pivot = _add_pivot_columns( + select(ranked.c.resource_id.label("resource_id")), + involved, + ranked.c.colname, + lambda field_name: getattr(ranked.c, value_attr_for(field_name)), + ) + pivot_cte = pivot.group_by(ranked.c.resource_id).cte("resource_pivot") + + # LEFT JOIN the licensed/coalesced pivot onto the membership universe: a + # resource with no licensed values keeps its row (null-shell) but is + # dropped by any field filter below. + resource_base = select(universe.c.resource_id) + for field_name in involved: + resource_base = resource_base.add_columns(pivot_cte.c[field_name]) + base = resource_base.select_from( + universe.outerjoin( + pivot_cte, pivot_cte.c.resource_id == universe.c.resource_id + ) + ).cte("resource_base") conditions = _build_field_conditions(base, params) stmt = select(base.c.resource_id)