speed up thumbnail queries and fix silent return of zero rows on failure

.github/workflows/docker.yml

@@ -22,6 +22,10 @@ jobs:
         else
           TAG=${GITHUB_REF#refs/heads/}
         fi
+        # Docker tags allow only [A-Za-z0-9_.-] (max 128, no leading . or -),
+        # so a branch like fix/foo would fail the build with "invalid
+        # reference format".
+        TAG=$(printf '%s' "$TAG" | sed -e 's/[^A-Za-z0-9_.-]/-/g' -e 's/^[.-]*//' | cut -c1-128)
         echo "tag=$TAG" >> $GITHUB_OUTPUT
 
     # Mirror what publish_to_pypi.yaml does so the docker image and the

src/test/test_neo4j_failure_no_silent_empty.py

@@ -0,0 +1,145 @@
+"""A failed Neo4j main query must never read as "no rows".
+
+Backend-free: ``vc`` is monkeypatched, so these run in CI without credentials.
+
+Why: from 2026-10-12 the TransgeneExpressionHere main query for mushroom body
+timed out on pdb. ``commit_list`` reports that by returning ``False``, which
+became an empty table, so the function returned ``{'count': 2339, 'rows': []}``
+— and the Solr result cache's ``count >= 0`` rule would have stored it.
+"""
+import types
+
+import pytest
+
+from vfbquery import cached_functions as cf
+from vfbquery import solr_result_cache as src
+from vfbquery import vfb_queries as vq
+
+
+def _count_reply(n):
+    return [{"columns": ["total_count"], "data": [{"row": [n]}]}]
+
+
+class _NC:
+    """Stand-in for ``vc.nc`` replaying one reply per ``commit_list`` call."""
+
+    def __init__(self, *replies):
+        self._replies = list(replies)
+
+    def commit_list(self, statements):
+        return self._replies.pop(0)
+
+
+def _wire(monkeypatch, *replies):
+    monkeypatch.setattr(vq, "vc", types.SimpleNamespace(
+        nc=_NC(*replies),
+        vfb=types.SimpleNamespace(oc=types.SimpleNamespace(
+            get_subclasses=lambda **kw: [])),
+    ))
+
+
+def test_transgene_expression_raises_when_main_query_fails(monkeypatch):
+    _wire(monkeypatch, _count_reply(2339), False)
+    with pytest.raises(vq.Neo4jQueryError, match="TransgeneExpressionHere main query"):
+        cf._original_get_transgene_expression_here(
+            "FBbt_00003748", return_dataframe=False, limit=10)
+
+
+def test_transgene_expression_raises_when_count_query_fails(monkeypatch):
+    _wire(monkeypatch, False)
+    with pytest.raises(vq.Neo4jQueryError, match="TransgeneExpressionHere count query"):
+        cf._original_get_transgene_expression_here(
+            "FBbt_00003748", return_dataframe=False, limit=10)
+
+
+def test_expression_overlaps_raises_when_main_query_fails(monkeypatch):
+    _wire(monkeypatch, _count_reply(79), False)
+    with pytest.raises(vq.Neo4jQueryError, match="ExpressionOverlapsHere main query"):
+        cf._original_get_expression_overlaps_here(
+            "VFBexp_FBtp0001321", return_dataframe=False, limit=5)
+
+
+def test_genuinely_empty_result_is_still_returned(monkeypatch):
+    empty = [{"columns": [], "data": []}]
+    _wire(monkeypatch, _count_reply(0), empty)
+    result = cf._original_get_transgene_expression_here(
+        "FBbt_00000000", return_dataframe=False, limit=10)
+    assert result["count"] == 0
+    assert result["rows"] == []
+
+
+@pytest.mark.parametrize("result, missing", [
+    ({"headers": {}, "rows": [], "count": 2339}, True),
+    ({"headers": {}, "rows": [], "count": 0}, False),
+    ({"headers": {}, "rows": [{"id": "x"}], "count": 2339}, False),
+    ({"headers": {}, "rows": [], "count": -1}, False),
+    ({"Id": "FBbt_00003748", "Name": "mushroom body"}, False),
+    ([], False),
+])
+def test_result_rows_missing(result, missing):
+    assert src.result_rows_missing(result) is missing
+
+
+def test_result_rows_missing_accepts_numpy_count():
+    np = pytest.importorskip("numpy")
+    assert src.result_rows_missing({"rows": [], "count": np.int64(5)}) is True
+
+
+# ---------------------------------------------------------------------------
+# read path: entries stored before the write-path guard existed
+# ---------------------------------------------------------------------------
+
+class _Solr:
+    """Answers cache lookups from a dict and records delete posts."""
+
+    def __init__(self, docs):
+        self.docs = docs
+        self.posts = []
+
+    def get(self, url, params=None, timeout=None):
+        doc_id = (params or {}).get("q", "").split("id:", 1)[-1]
+        payload = self.docs.get(doc_id)
+        docs = [{"cache_data": payload}] if payload is not None else []
+        return types.SimpleNamespace(status_code=200,
+                                     json=lambda: {"response": {"docs": docs}})
+
+    def post(self, url, data=None, headers=None, params=None, timeout=None):
+        self.posts.append(data)
+        return types.SimpleNamespace(status_code=200, text="", json=lambda: {})
+
+
+def _cache_with(monkeypatch, result):
+    import json
+    for var in ("VFBQUERY_CACHE_NAMESPACE", "VFBQUERY_CACHE_NAMESPACE_FALLBACK",
+                "VFBQUERY_CACHE_READONLY", "VFBQUERY_CACHE_ENABLED"):
+        monkeypatch.delenv(var, raising=False)
+    cache = src.SolrResultCache()
+    monkeypatch.setattr(cache, "_solr_available", lambda: True)
+    payload = src._encode_cache_field(json.dumps(
+        cache._create_cache_metadata(result, limit=-1)))
+    solr = _Solr({"vfb_query_transgene_expression_here_T": payload})
+    monkeypatch.setattr(src.requests, "get", solr.get)
+    monkeypatch.setattr(src.requests, "post", solr.post)
+    return cache, solr
+
+
+def test_cached_result_with_missing_rows_is_a_miss_and_purged(monkeypatch):
+    # Stored 2026-10-15 for mushroom body by the pre-fix code, served to every
+    # PR check until it expired.
+    cache, solr = _cache_with(monkeypatch, {"headers": {}, "rows": [], "count": 2339})
+    assert cache.get_cached_result("transgene_expression_here", "T", limit=-1) is None
+    assert any("vfb_query_transgene_expression_here_T" in p for p in solr.posts)
+
+
+def test_cached_result_with_missing_rows_is_not_purged_when_read_only(monkeypatch):
+    cache, solr = _cache_with(monkeypatch, {"headers": {}, "rows": [], "count": 2339})
+    monkeypatch.setenv("VFBQUERY_CACHE_READONLY", "true")
+    assert cache.get_cached_result("transgene_expression_here", "T", limit=-1) is None
+    assert solr.posts == []
+
+
+def test_cached_result_with_rows_is_served(monkeypatch):
+    good = {"headers": {}, "rows": [{"id": "VFBexp_1"}], "count": 2339}
+    cache, solr = _cache_with(monkeypatch, good)
+    assert cache.get_cached_result("transgene_expression_here", "T", limit=-1) == good
+    assert solr.posts == []

src/vfbquery/solr_result_cache.py

@@ -166,6 +166,25 @@ def result_is_partial(result) -> bool:
     return isinstance(result, dict) and bool(result.get(PARTIAL_RESULT_KEY))
 
 
+def result_rows_missing(result) -> bool:
+    """True for a dict result that reports rows (count > 0) but carries none.
+
+    That is the shape of a query whose count query succeeded and whose main
+    query failed: Neo4j's ``commit_list`` returns ``False``, which becomes an
+    empty table. ``count >= 0`` alone would store it and serve "no rows" for
+    the cache TTL (TransgeneExpressionHere on mushroom body, 2026-10).
+    """
+    if not isinstance(result, dict):
+        return False
+    rows = result.get('rows')
+    if not isinstance(rows, list) or rows:
+        return False
+    try:
+        return int(result.get('count', 0)) > 0
+    except (TypeError, ValueError):
+        return False
+
+
 def preview_is_resolved(query: Dict[str, Any]) -> bool:
     """True when a query's preview holds a final answer.
 
@@ -592,7 +611,18 @@ def _get_cached_result_in(self, namespace: str, allow_purge: bool,
             if cached_limit != -1:
                 logger.debug(f"Cache miss: Unexpected cached result with limit={cached_limit}, expected -1")
                 return None
-
+
+            # Entries stored before result_rows_missing() guarded the write
+            # path can still hold count > 0 with no rows (a failed main query).
+            # Treat them as a miss and purge them so the next writable call
+            # recomputes and overwrites them, rather than serving "no rows"
+            # until the entry expires.
+            if result_rows_missing(cached_data.get("result")):
+                logger.warning(
+                    f"Cached result for {query_type}({term_id}) has count > 0 but no rows; clearing it")
+                purge(cache_doc_id)
+                return None
+
             # If requesting unlimited results, return the full cached result
             if requested_limit == -1:
                 result = cached_data["result"]
@@ -1440,6 +1470,10 @@ def _call(*call_args, **call_kwargs):
                             full_is_valid = False
                             logger.warning(
                                 f"Not caching partial result for {query_type}({term_id})")
+                        if full_is_valid and result_rows_missing(full_result):
+                            full_is_valid = False
+                            logger.warning(
+                                f"Not caching result with count > 0 but no rows for {query_type}({term_id})")
                     elif isinstance(full_result, (list, str)):
                         full_is_valid = len(full_result) > 0
                     else:
@@ -1526,6 +1560,10 @@ def _call(*call_args, **call_kwargs):
                             result_is_valid = False
                             logger.warning(
                                 f"Not caching partial result for {query_type}({term_id})")
+                        if result_is_valid and result_rows_missing(result):
+                            result_is_valid = False
+                            logger.warning(
+                                f"Not caching result with count > 0 but no rows for {query_type}({term_id})")
                     elif isinstance(result, (list, str)):
                         result_is_valid = len(result) > 0
                     else:

src/vfbquery/vfb_queries.py

@@ -144,6 +144,26 @@ class ConnectivityBackendError(RuntimeError):
     """
 
 
+class Neo4jQueryError(RuntimeError):
+    """A Neo4j query errored or timed out instead of returning a result.
+
+    ``commit_list`` signals failure by returning ``False``, and
+    ``get_dict_cursor`` turns that into ``[]``, so a timed-out main query used
+    to come back as ``{'count': N, 'rows': []}``. That is a valid result to the
+    Solr cache (``count >= 0``) and reads as "no data" to every caller
+    (TransgeneExpressionHere on mushroom body, 2026-10). Same rationale as
+    :class:`ConnectivityBackendError`: raise rather than report zero rows.
+    """
+
+
+def _commit_or_raise(query, description):
+    """``vc.nc.commit_list([query])``, raising :class:`Neo4jQueryError` on failure."""
+    results = vc.nc.commit_list([query])
+    if not isinstance(results, list):
+        raise Neo4jQueryError(f"Neo4j query failed: {description}")
+    return results
+
+
 #: Result key set by :func:`get_downstream_class_connectivity` /
 #: :func:`get_upstream_class_connectivity` when the aggregation ran but some
 #: queried instances contributed nothing (their per-instance connectivity
@@ -3913,7 +3933,7 @@ def get_expression_overlaps_here(expression_pattern_short_form: str, return_data
         RETURN COUNT(DISTINCT anat) AS total_count
     """
 
-    count_results = vc.nc.commit_list([count_query])
+    count_results = _commit_or_raise(count_query, f"ExpressionOverlapsHere count query for {expression_pattern_short_form}")
     count_df = pd.DataFrame.from_records(get_dict_cursor()(count_results))
     total_count = count_df['total_count'][0] if not count_df.empty else 0
 
@@ -3961,16 +3981,26 @@ def get_expression_overlaps_here(expression_pattern_short_form: str, return_data
         }}
         CALL {{
             WITH anat
-            OPTIONAL MATCH (anat)<-[:has_source|SUBCLASSOF|INSTANCEOF*]-(i:Individual)<-[:depicts]-(channel:Individual)-[irw:in_register_with]->(template:Individual)-[:depicts]->(template_anat:Individual)
-            OPTIONAL MATCH (channel)-[:is_specified_output_of]->(technique:Class)
-            // Do NOT filter `i IS NOT NULL` here: this is a CALL subquery, so an
-            // input anat for which it yields no rows is dropped entirely. Anatomy
-            // classes rarely have this image path, so the filter emptied the whole
+            // apoc.path.subgraphNodes instead of a `[...*]` var-length
+            // pattern — see get_transgene_expression_here. The `*` form timed
+            // out for every anat in the AnatomyExpressedIn tests (2026-10);
+            // this returns in well under a second even for GABAergic neuron
+            // (~110k reachable nodes), since LIMIT 5 stops the walk early.
+            //
+            // Do NOT let an anat without images drop out of the result: anatomy
+            // classes rarely have this image path, so doing so emptied the whole
             // table (count 79 -> 0 rows) and desynced it from count_query. The
-            // all-null row is stripped to '' by the REPLACE below, so the anat
-            // still returns a row (empty thumbnail/template/technique).
-            WITH anat, i, template_anat, technique, irw LIMIT 5
-            WITH anat, collect({{i: i, template_anat: template_anat, technique: technique, irw: irw}}) AS imgs
+            // inner collect() always yields one row (imgs = [] when there is no
+            // image), so the anat keeps its row with an empty
+            // thumbnail/template/technique.
+            CALL {{
+                WITH anat
+                CALL apoc.path.subgraphNodes(anat, {{relationshipFilter: '<has_source|<SUBCLASSOF|<INSTANCEOF', minLevel: 1}}) YIELD node AS i
+                MATCH (i:Individual)<-[:depicts]-(channel:Individual)-[irw:in_register_with]->(template:Individual)-[:depicts]->(template_anat:Individual)
+                OPTIONAL MATCH (channel)-[:is_specified_output_of]->(technique:Class)
+                WITH i, template_anat, technique, irw LIMIT 5
+                RETURN collect({{i: i, template_anat: template_anat, technique: technique, irw: irw}}) AS imgs
+            }}
             WITH anat, imgs, head(imgs) AS rep
             RETURN
                 rep.template_anat AS template_anat,
@@ -3995,7 +4025,7 @@ def get_expression_overlaps_here(expression_pattern_short_form: str, return_data
             thumbnail
     """
 
-    results = vc.nc.commit_list([main_query])
+    results = _commit_or_raise(main_query, f"ExpressionOverlapsHere main query for {expression_pattern_short_form}")
     df = pd.DataFrame.from_records(get_dict_cursor()(results))
 
     if not df.empty:
@@ -7453,7 +7483,7 @@ def get_transgene_expression_here(anatomy_short_form: str, return_dataframe=True
         WHERE anat.short_form IN {anat_short_forms!r}
         RETURN COUNT(DISTINCT ep) AS total_count
     """
-    count_results = vc.nc.commit_list([count_query])
+    count_results = _commit_or_raise(count_query, f"TransgeneExpressionHere count query for {anatomy_short_form}")
     count_df = pd.DataFrame.from_records(get_dict_cursor()(count_results))
     total_count = count_df['total_count'][0] if not count_df.empty else 0
 
@@ -7538,19 +7568,29 @@ def get_transgene_expression_here(anatomy_short_form: str, return_dataframe=True
         }}
         CALL {{
             WITH ep
-            OPTIONAL MATCH (ep)<-[:has_source|SUBCLASSOF|INSTANCEOF*]-(i:Individual)<-[:depicts]-(channel:Individual)-[irw:in_register_with]->(:Template)-[:depicts]->(templ:Template)
-            OPTIONAL MATCH (channel)-[:is_specified_output_of]->(technique:Class)
-            // Do NOT filter `i IS NOT NULL` here: this is a CALL subquery, so an
-            // input ep for which it yields no rows is dropped from the result
-            // entirely. Filtering image-less EPs desynced the table from
-            // count_query (which has no image filter) and silently hid every
-            // expression pattern with no aligned image — e.g. splits targeting a
-            // neuron that carry no registered image. When there is no image the
-            // OPTIONAL MATCH yields one all-null row; the null placeholders are
-            // stripped to '' by the REPLACE below, so the ep still returns a row
-            // (empty thumbnail/template/technique) and stays in the results.
-            WITH ep, i, templ, technique, irw LIMIT 5
-            WITH ep, collect({{i: i, templ: templ, technique: technique, irw: irw}}) AS imgs
+            // Walk the has_source/SUBCLASSOF/INSTANCEOF closure with
+            // apoc.path.subgraphNodes rather than a `[...*]` var-length
+            // pattern. Both reach the same image Individuals, but the planner
+            // expanded `*` from the Template end and enumerated every path,
+            // taking ~37s for 10 mushroom-body EPs and timing out at
+            // limit=-1 (2026-10). subgraphNodes is anchored at ep and visits
+            // each node once, with no depth cap.
+            //
+            // Do NOT let an image-less ep drop out of the result: filtering
+            // them desynced the table from count_query (which has no image
+            // filter) and silently hid e.g. splits targeting a neuron that
+            // carry no registered image. The collect() below is an aggregate
+            // with no grouping key, so this subquery always yields one row
+            // (imgs = [] when there is no image) and the ep keeps its row
+            // with an empty thumbnail/template/technique.
+            CALL {{
+                WITH ep
+                CALL apoc.path.subgraphNodes(ep, {{relationshipFilter: '<has_source|<SUBCLASSOF|<INSTANCEOF', minLevel: 1}}) YIELD node AS i
+                MATCH (i:Individual)<-[:depicts]-(channel:Individual)-[irw:in_register_with]->(:Template)-[:depicts]->(templ:Template)
+                OPTIONAL MATCH (channel)-[:is_specified_output_of]->(technique:Class)
+                WITH i, templ, technique, irw LIMIT 5
+                RETURN collect({{i: i, templ: templ, technique: technique, irw: irw}}) AS imgs
+            }}
             WITH ep, imgs, head(imgs) AS rep
             RETURN
                 rep.templ AS templ,
@@ -7575,7 +7615,7 @@ def get_transgene_expression_here(anatomy_short_form: str, return_dataframe=True
             thumbnail
     """
 
-    results = vc.nc.commit_list([main_query])
+    results = _commit_or_raise(main_query, f"TransgeneExpressionHere main query for {anatomy_short_form}")
     df = pd.DataFrame.from_records(get_dict_cursor()(results))
     if not df.empty:
         df = encode_markdown_links(df, ['name', 'expressed_in', 'template', 'thumbnail'])