Bump mshick/add-pr-comment from 3.9.0 to 3.11.0

.github/workflows/branch.yml

@@ -20,7 +20,7 @@ jobs:
       # NOTE - this doesn't currently work if the PR is coming from a fork, due to limitations in GitHub actions secrets
       - name: Post PR comment
         if: failure()
-        uses: mshick/add-pr-comment@ffd016c7e151d97d69d21a843022fd4cd5b96fe5 # v3.9.0
+        uses: mshick/add-pr-comment@8e4927817251f1ff60c001f04568532b38e0b4a0 # v3.11.0
         with:
           message: |
             ## This PR is against the `main` branch :x:

pyproject.toml

@@ -9,7 +9,7 @@ readme = "README.md"
 [tool.poetry.dependencies]
 python = "^3.11"
 # use `rev` to pin to a specific hash, or tag, from the git repo
-labretriever = {git = "https://github.com/cmatKhan/labretriever.git", rev = "72bbf3b"}
+labretriever = {git = "https://github.com/cmatKhan/labretriever.git", rev = "ecaa7e3"}
 shiny = "^1.4.0"
 shinywidgets = "^0.7.1"
 upsetjs-jupyter-widget = "^1.9.0"

tests/unit/test_sample_conditions.py

@@ -0,0 +1,126 @@
+"""Unit tests for ``tfbpshiny.utils.sample_conditions``."""
+
+from __future__ import annotations
+
+from typing import Any
+
+import pandas as pd
+import pytest
+
+from tfbpshiny.utils.sample_conditions import (
+    build_condition_label,
+    fetch_sample_condition_map,
+)
+
+# ---------- build_condition_label ----------
+
+
+@pytest.mark.parametrize(
+    "values,expected",
+    [
+        (["YPD"], "YPD"),
+        (["ZEV", "P", "45"], "ZEV / P / 45"),
+        (["ZEV", None, "45"], "ZEV / 45"),
+        ([None, None], ""),
+        ([], ""),
+        (["  spaced  "], "spaced"),
+        (["", "  ", "YPD"], "YPD"),
+        (["nan", "NaN", "YPD"], "YPD"),
+        ([float("nan"), "YPD"], "YPD"),
+        ([1, 2.5, "x"], "1 / 2.5 / x"),
+    ],
+)
+def test_build_condition_label(values: list[Any], expected: str) -> None:
+    assert build_condition_label(values) == expected
+
+
+# ---------- fetch_sample_condition_map ----------
+
+
+class _StubVdb:
+    """Minimal stub capturing the last-issued SQL and returning a canned DataFrame."""
+
+    def __init__(self, df: pd.DataFrame) -> None:
+        self._df = df
+        self.last_sql: str | None = None
+
+    def query(self, sql: str, **params: Any) -> pd.DataFrame:
+        self.last_sql = sql
+        return self._df
+
+
+def test_fetch_sample_condition_map_empty_cols_returns_empty() -> None:
+    vdb = _StubVdb(pd.DataFrame())
+    assert fetch_sample_condition_map(vdb, "anything", []) == {}
+    # No query should have been issued.
+    assert vdb.last_sql is None
+
+
+def test_fetch_sample_condition_map_single_column() -> None:
+    df = pd.DataFrame(
+        {
+            "sample_id": ["s1", "s2", "s3"],
+            "Experimental condition": ["YPD", "HEAT", None],
+        }
+    )
+    vdb = _StubVdb(df)
+    result = fetch_sample_condition_map(vdb, "harbison", ["Experimental condition"])
+
+    assert result == {"s1": "YPD", "s2": "HEAT"}
+    # s3 had an all-NULL label and must not be present.
+    assert "s3" not in result
+    # Column with a space is double-quoted in the SQL.
+    assert vdb.last_sql is not None
+    assert '"Experimental condition"' in vdb.last_sql
+    assert "FROM harbison_meta" in vdb.last_sql
+
+
+def test_fetch_sample_condition_map_multi_column_joined() -> None:
+    df = pd.DataFrame(
+        {
+            "sample_id": ["s1", "s2"],
+            "mechanism": ["ZEV", "GEV"],
+            "restriction": ["P", None],
+        }
+    )
+    vdb = _StubVdb(df)
+    result = fetch_sample_condition_map(vdb, "hackett", ["mechanism", "restriction"])
+
+    assert result == {"s1": "ZEV / P", "s2": "GEV"}
+
+
+def test_fetch_sample_condition_map_non_string_sample_id_coerced() -> None:
+    # sample_id comes back from DuckDB as int or bytes in some schemas.
+    df = pd.DataFrame({"sample_id": [1, 2], "cond": ["A", "B"]})
+    vdb = _StubVdb(df)
+    result = fetch_sample_condition_map(vdb, "ds", ["cond"])
+    assert result == {"1": "A", "2": "B"}
+
+
+@pytest.mark.parametrize(
+    "bad_name",
+    [
+        "foo; DROP TABLE users",
+        "foo-bar",
+        "foo bar",
+        "1foo",
+        "",
+        'foo"; SELECT',
+    ],
+)
+def test_fetch_sample_condition_map_rejects_unsafe_db_name(bad_name: str) -> None:
+    vdb = _StubVdb(pd.DataFrame({"sample_id": ["s1"], "cond": ["A"]}))
+    with pytest.raises(ValueError, match="safe SQL identifier"):
+        fetch_sample_condition_map(vdb, bad_name, ["cond"])
+
+
+def test_fetch_sample_condition_map_escapes_embedded_quotes_in_cols() -> None:
+    # Column name with an embedded double-quote must be escaped as "" per
+    # DuckDB identifier rules; otherwise the SQL would break out of quoting.
+    df = pd.DataFrame({"sample_id": ["s1"], 'weird"name': ["A"]})
+    vdb = _StubVdb(df)
+    result = fetch_sample_condition_map(vdb, "ds", ['weird"name'])
+    assert result == {"s1": "A"}
+    assert vdb.last_sql is not None
+    # The emitted identifier is "weird""name" — one escaped embedded quote.
+    assert '"weird""name"' in vdb.last_sql

tfbpshiny/app.py

@@ -137,6 +137,7 @@ def app_server(input: Any, output: Any, session: Any) -> None:
         col_preference=col_preference,
         dataset_filters=dataset_filters,
         vdb=vdb,
+        app_datasets=app_datasets,
         logger=logger,
     )
 
@@ -154,6 +155,7 @@ def app_server(input: Any, output: Any, session: Any) -> None:
         col_preference=col_preference_p,
         dataset_filters=dataset_filters,
         vdb=vdb,
+        app_datasets=app_datasets,
         logger=logger,
     )
 

tfbpshiny/modules/binding/page_test.py

@@ -24,6 +24,7 @@
     binding_workspace_server,
 )
 from tfbpshiny.modules.binding.ui import binding_sidebar_ui, binding_workspace_ui
+from tfbpshiny.utils.vdb_init import AppDatasets
 
 logger = logging.getLogger("shiny")
 
@@ -157,6 +158,7 @@ def active_binding_datasets() -> list[str]:
         col_preference=col_preference,
         dataset_filters=_dataset_filters,
         vdb=vdb,
+        app_datasets=AppDatasets(condition_cols={}, upstream_cols={}),
         logger=logger,
     )
 

tfbpshiny/modules/binding/server/workspace.py

@@ -2,6 +2,7 @@
 
 import itertools
 from collections.abc import Callable
+from html import escape
 from logging import Logger
 from typing import Any, Literal
 
@@ -16,7 +17,8 @@
     get_measurement_column,
     regulator_scatter_sql,
 )
-from tfbpshiny.utils.vdb_init import get_regulator_display_name
+from tfbpshiny.utils.sample_conditions import fetch_sample_condition_map
+from tfbpshiny.utils.vdb_init import AppDatasets, get_regulator_display_name
 
 
 @module.server
@@ -29,6 +31,7 @@ def binding_workspace_server(
     col_preference: Callable[[], str],
     dataset_filters: reactive.Value[dict[str, Any]],
     vdb: VirtualDB,
+    app_datasets: AppDatasets,
     logger: Logger,
 ) -> None:
     """
@@ -46,6 +49,35 @@ def binding_workspace_server(
         zip(_reg_df["regulator_locus_tag"], _reg_df["display_name"])
     )
 
+    @reactive.calc
+    def _condition_maps() -> dict[str, dict[str, str]]:
+        """
+        ``{db_name: {sample_id: label}}`` for each active dataset that has
+        experimental_condition columns.
+
+        Used to annotate tooltips on the selected-regulator overlay in the
+        distribution plot so the user can distinguish multiple samples of the
+        same regulator. Datasets without ``condition_cols`` are omitted from
+        the outer dict, causing the tooltip to skip their side.
+
+        :trigger active_binding_datasets: re-runs when the user toggles a
+            binding dataset on or off.
+        :returns: Outer dict keyed by db_name; inner dict maps sample_id to
+            the joined condition label.
+
+        """
+        out: dict[str, dict[str, str]] = {}
+        for db in active_binding_datasets():
+            cols = app_datasets.condition_cols.get(db, [])
+            if not cols:
+                continue
+            try:
+                out[db] = fetch_sample_condition_map(vdb, db, cols)
+            except Exception:
+                logger.exception("Failed to fetch condition map for %s", db)
+                out[db] = {}
+        return out
+
     @reactive.calc
     def _pairs() -> list[tuple[str, str]]:
         """
@@ -169,6 +201,8 @@ def distributions_plot() -> ui.Tag:
         except Exception:
             selected_reg = ""
 
+        cond_maps = _condition_maps()
+
         # Build a single combined box trace using x as the category axis.
         # Each point's x value is the pair label; Plotly groups points under
         # each category and draws one box per unique x value.
@@ -185,10 +219,15 @@ def distributions_plot() -> ui.Tag:
             label_a = display_names.get(db_a, db_a)
             label_b = display_names.get(db_b, db_b)
             pair_label = f"{label_a}<br>vs<br>{label_b}"
+            cond_a = cond_maps.get(db_a, {})
+            cond_b = cond_maps.get(db_b, {})
             if not df.empty:
                 df_clean = df.dropna(subset=["correlation"])
-                for tag, corr in zip(
-                    df_clean["regulator_locus_tag"], df_clean["correlation"]
+                for tag, corr, sample_a, sample_b in zip(
+                    df_clean["regulator_locus_tag"],
+                    df_clean["correlation"],
+                    df_clean["db_a_id"],
+                    df_clean["db_b_id"],
                 ):
                     display = sym_map.get(tag, tag)
                     all_x.append(pair_label)
@@ -198,7 +237,24 @@ def distributions_plot() -> ui.Tag:
                     if tag == selected_reg:
                         sel_x.append(pair_label)
                         sel_y.append(corr)
-                        sel_hover.append(display)
+                        # Per-dot hover: regulator + r + one condition line per
+                        # dataset that has a non-empty label for this sample.
+                        # All DB-sourced strings are HTML-escaped before being
+                        # joined with the <br> separators because Plotly renders
+                        # hovertext as HTML (stored-XSS sink if any researcher-
+                        # uploaded metadata ever contained markup).
+                        hover_lines = [escape(display), f"r = {corr:.3f}"]
+                        label_sample_a = cond_a.get(str(sample_a), "")
+                        if label_sample_a:
+                            hover_lines.append(
+                                f"{escape(label_a)}: {escape(label_sample_a)}"
+                            )
+                        label_sample_b = cond_b.get(str(sample_b), "")
+                        if label_sample_b:
+                            hover_lines.append(
+                                f"{escape(label_b)}: {escape(label_sample_b)}"
+                            )
+                        sel_hover.append("<br>".join(hover_lines))
                         sel_tags.append(tag)
 
         fig.add_trace(
@@ -224,9 +280,9 @@ def distributions_plot() -> ui.Tag:
                     x=sel_x,
                     y=sel_y,
                     mode="markers",
-                    text=sel_hover,
+                    hovertext=sel_hover,
                     customdata=sel_tags,
-                    hovertemplate="%{text}<br>r = %{y:.3f}<extra></extra>",
+                    hovertemplate="%{hovertext}<extra></extra>",
                     marker=dict(size=10, color="black", symbol="circle"),
                     showlegend=False,
                 )

tfbpshiny/modules/perturbation/page_test.py

@@ -28,6 +28,7 @@
     perturbation_sidebar_ui,
     perturbation_workspace_ui,
 )
+from tfbpshiny.utils.vdb_init import AppDatasets
 
 logger = logging.getLogger("shiny")
 
@@ -165,6 +166,7 @@ def active_perturbation_datasets() -> list[str]:
         col_preference=col_preference,
         dataset_filters=_dataset_filters,
         vdb=vdb,
+        app_datasets=AppDatasets(condition_cols={}, upstream_cols={}),
         logger=logger,
     )