fix type for advanced freetext and allow free-text for Item search #263

vincentsarago · 2025-07-10T10:05:06Z

To Do

add tests

vincentsarago · 2025-07-24T14:32:03Z

stac_fastapi/pgstac/core.py

@@ -54,8 +54,7 @@ async def all_collections(  # noqa: C901
        sortby: Optional[str] = None,
        filter_expr: Optional[str] = None,
        filter_lang: Optional[str] = None,
-        q: Optional[List[str]] = None,
-        **kwargs,
+        **kwargs: Any,


In this PR we changed and we now forward kwargs to _clean_search_args method.

What happens if POST /search with {"q": 123} is submitted? Does q make it all the way to the DB and raises due to invalid types? There won't be any early API model validation of the parameter?

vincentsarago · 2025-07-24T14:56:51Z

@fmigneault could you check this PR 🙏

Overall is to do as #267

Instead of adding q to the method annotation, this PR forward any kwargs to the _clean_search_args function

gadomski · 2025-07-24T16:18:03Z

tests/conftest.py

+        pgdatabase=database.dbname,
+    )
+    logger.info("Creating app Fixture")
+    time.time()


Is time used for anything?

gadomski · 2025-07-24T16:18:51Z

tests/resources/test_collection.py

+
+    resp = await app_client.get(
+        "/collections",
+        params={"q": "temperature,yo"},


fmigneault · 2025-07-24T16:44:42Z

tests/resources/test_collection.py

There are no tests for the /search?q= and /collections/{col}/items?q=... cases?

Also, there could be POST /search with {"q": "advanced AND search"} or {"q": ["basic", "search"]}.

I don't believe this PR is meant to implement free-text item search, just correct some stuff around free-text collection search.

Function self._clean_search_args calls are updated with the **kwargs, so q will now trickle down within these calls as well (which is good), and should be considered (but could be in a separate PR though not to block this one).

q won't be in /search because we don't usually use the free-text extension for items

if you're passing a dict, pydantic should raise a validation https://github.com/stac-utils/stac-fastapi/blob/fa42985255fad0bab7dbe3aadbf1f74cb1635f3a/stac_fastapi/extensions/stac_fastapi/extensions/core/free_text/request.py#L37-L43

I don't believe this PR is meant to implement free-text item search, just correct some stuff around free-text collection search.

Exactly, but it also enables free-text for items by using kwargs, as for other unknown extension people would want to implement. Now they would have to just pass a custom _clean_search_args to support any kind of input passed through kwargs

OK. Good point about the Pydantic model.

I'm not sure to understand the "q won't be in /search".
Isn't it available if the conformance is applied?
https://github.com/stac-utils/stac-fastapi/blob/fa42985255fad0bab7dbe3aadbf1f74cb1635f3a/stac_fastapi/extensions/stac_fastapi/extensions/core/free_text/free_text.py#L27-L40

I was able to activate it is my implementation (https://github.com/crim-ca/stac-app/pull/28/files). It should do the title/description/keywords free-text search across all collections' items, as if filter or query was used, no?

I'm not sure to understand the "q won't be in /search".

I just mean that q parameter will ONLY be available if enabled at the application level.

As mentioned in #263 (comment) it works only for Advanced because we don't do str -> list -> str transformation but keep the values as str

fmigneault · 2025-07-24T16:45:43Z

stac_fastapi/pgstac/core.py

@@ -54,8 +54,7 @@ async def all_collections(  # noqa: C901
        sortby: Optional[str] = None,
        filter_expr: Optional[str] = None,
        filter_lang: Optional[str] = None,
-        q: Optional[List[str]] = None,
-        **kwargs,
+        **kwargs: Any,


What happens if POST /search with {"q": 123} is submitted? Does q make it all the way to the DB and raises due to invalid types? There won't be any early API model validation of the parameter?

vincentsarago · 2025-07-25T08:05:15Z

😭 Well in fact I added tests for /search and it shows a bug

When we are using _clean_search_args we're transforming a list to a string which is what pgstac expect apparently. But for /search what will happen is:

user pass q=temperature,yo
fastapi will convert this string to ["temperature","yo"] using https://github.com/stac-utils/stac-fastapi/blob/fa42985255fad0bab7dbe3aadbf1f74cb1635f3a/stac_fastapi/extensions/stac_fastapi/extensions/core/free_text/request.py#L30-L34
we go through _clean_search_args and transform it back to string: "temperature OR yo"
we do put back the cleaned parameter into the PgStacSearch pydantic model

stac-fastapi-pgstac/stac_fastapi/pgstac/core.py

Line 402 in bbf0cb5

search_request = self.pgstac_search_model(**clean)

but PgStacSearch expect a List[str] (as defined in the extension)

fmigneault · 2025-07-25T14:13:46Z

fastapi will convert this string to ["temperature","yo"]

Exactly. And this is invalid according to Advanced free-text. The comma is plain-text in this variant. It should split and do OR only for Basic free-text.

That being said, I would love for Advanced spec to be updated and align it with Basic to avoid this ambiguity. This is pretty much what every open issues requests:

vincentsarago · 2025-07-28T07:09:15Z

Exactly. And this is invalid according to Advanced free-text. The comma is plain-text in this variant. It should split and do OR only for Basic free-text.

There will be no issue when working with advanced Free Search, the param will always be in string and no transformation will be done. This bug only happens for basic free search because we have a list in input (as defined by the spec) but we need to pass a string to PgSTAC.

vincentsarago · 2025-07-28T09:00:12Z

I think the main issue it that the spec define both string and list of string for input (GET and POST) so IMO, pgstac should be able to handle both.

I'm going to see if we can do this in pgstac instead of having hacks in stac-fastapi-pgstac`

vincentsarago · 2025-08-01T08:51:12Z

stac_fastapi/pgstac/core.py

+            # join the list[str] with ` OR `
+            # ref: https://github.com/stac-utils/stac-fastapi-pgstac/pull/263
+            if q := clean_args.pop("q", None):
+                clean_args["q"] = " OR ".join(q) if isinstance(q, list) else q


We need custom code to handle list[str] passed by collection-search Free-Text extension as pgstac will only accept str

Note: we don't need this in items search because we will use pydantic serialization

vincentsarago · 2025-08-01T08:52:57Z

stac_fastapi/pgstac/extensions/free_text.py

+    ] = Field(
+        None,
+        description="Parameter to perform free-text queries against STAC metadata",
+    )


Custom FreeTextExtensionPostRequest model which will handle JSON serialization, transforming list[str] to str

vincentsarago · 2025-08-01T08:54:34Z

tests/data/test_item.json

@@ -34,6 +34,7 @@
    "type": "Polygon"
  },
  "properties": {
+    "description": "Landat 8 imagery radiometrically calibrated and orthorectified using gound points and Digital Elevation Model (DEM) data to correct relief displacement.",


free-text for items will only work within properties (title, description keywords)

https://github.com/stac-utils/pgstac/blob/45ac2478b58946529872ec3feed0ee0c838c4742/src/pgstac/sql/004_search.sql#L225-L227

Not relevant for this PR, but this is why item-level free-text search feels funny to me ... IMO this info should live at the collection level only.

In the case of a collection where each item contains relatively the same information at different place/time, Item-level free-text search is indeed redundant. However, imagine the case of a collection regrouping multiple "conceptual" items, such as many AI models described using MLM extension. In this case, each Item could contain different descriptions and keywords within the same collection, which makes the free-text search very relevant at item-level.

gadomski · 2025-08-01T14:47:06Z

tests/data/test_item.json

@@ -34,6 +34,7 @@
    "type": "Polygon"
  },
  "properties": {
+    "description": "Landat 8 imagery radiometrically calibrated and orthorectified using gound points and Digital Elevation Model (DEM) data to correct relief displacement.",


Not relevant for this PR, but this is why item-level free-text search feels funny to me ... IMO this info should live at the collection level only.

vincentsarago · 2025-08-05T20:59:55Z

are we good to merge this one @bitner @fmigneault ?

fix type for advanced freetext

2fd8c8f

vincentsarago mentioned this pull request Jul 10, 2025

fix extension free-text advanced GET-q query not split by comma stac-utils/stac-fastapi#849

Closed

4 tasks

fmigneault added a commit to crim-ca/stac-fastapi-pgstac that referenced this pull request Jul 16, 2025

add missing q param for item search and collection-items search (rela…

6e8d9dd

…tes to stac-utils#263)

fmigneault mentioned this pull request Jul 16, 2025

add missing q param for item search and collection-items search #267

Closed

4 tasks

update from main

bb91b68

vincentsarago commented Jul 24, 2025

View reviewed changes

add tests and remove free-text from method annotations

c0767f3

vincentsarago force-pushed the patch/allow-advanced-free-text-ext branch from 4f90e88 to c0767f3 Compare July 24, 2025 14:32

add advanced tests

bbf0cb5

vincentsarago requested review from alukach and gadomski and removed request for alukach July 24, 2025 14:54

gadomski approved these changes Jul 24, 2025

View reviewed changes

fmigneault reviewed Jul 24, 2025

View reviewed changes

add failing tests

b5fdaba

fix and enable free-text for items

947a977

vincentsarago commented Aug 1, 2025

View reviewed changes

vincentsarago changed the title ~~fix type for advanced freetext~~ fix type for advanced freetext and allow free-text for Item search Aug 1, 2025

vincentsarago requested review from gadomski and bitner August 1, 2025 08:55

gadomski approved these changes Aug 1, 2025

View reviewed changes

Merge branch 'main' into patch/allow-advanced-free-text-ext

eaf6dd8

vincentsarago mentioned this pull request Aug 7, 2025

release: v6.0.0 #277

Merged

Merge branch 'main' into patch/allow-advanced-free-text-ext

af4b331

vincentsarago merged commit 8e5ebfa into main Aug 8, 2025
7 checks passed

vincentsarago deleted the patch/allow-advanced-free-text-ext branch August 8, 2025 09:04

fmigneault mentioned this pull request Aug 14, 2025

Fix free text basic search post #282

Closed

4 tasks

fix type for advanced freetext and allow free-text for Item search #263

fix type for advanced freetext and allow free-text for Item search #263

Uh oh!

Conversation

vincentsarago commented Jul 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

To Do

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vincentsarago commented Jul 24, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gadomski Jul 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vincentsarago commented Jul 25, 2025

Uh oh!

fmigneault commented Jul 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

vincentsarago commented Jul 28, 2025

Uh oh!

vincentsarago commented Jul 28, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

vincentsarago commented Aug 5, 2025

Uh oh!

Uh oh!

Uh oh!

vincentsarago commented Jul 10, 2025 •

edited

Loading

gadomski Jul 24, 2025 •

edited

Loading

fmigneault commented Jul 25, 2025 •

edited

Loading