refactor: rename a run's variable values to cells

api/README.md

@@ -58,12 +58,12 @@ This returns a JSON snapshot for the proposal:
 ### 2. Query the runs
 
 For instance, to fetch the first 10 runs of proposal `2956` along with all
-of their variables:
+of their cells:
 
 ```gql
 query TableDataQuery($per_page: Int = 10) {
   runs(database: {proposal: "2956"}, per_page: $per_page) {
-    variables {
+    cells {
       name
       value
       dtype
@@ -72,13 +72,13 @@ query TableDataQuery($per_page: Int = 10) {
 }
 ```
 
-Each run is returned as a flat list of `DamnitVariable` entries (`name`,
+Each run is returned as a flat list of `Cell` entries (`name`,
 `value`, `dtype`). One can pass a list of `names` to select variables:
 
 ```gql
 query TableDataQuery($per_page: Int = 10) {
   runs(database: {proposal: "2956"}, per_page: $per_page) {
-    variables(names: ["proposal", "run"]) {
+    cells(names: ["proposal", "run"]) {
       name
       value
       dtype

api/src/damnit_api/graphql/directives.py

@@ -1,7 +1,7 @@
 import strawberry
 from strawberry.directive import DirectiveLocation, DirectiveValue
 
-from ..runs.types import DamnitRun, DamnitVariable
+from ..runs.types import Cell, DamnitRun
 from ..shared.const import DamnitType
 
 HEAVY_DATA = (
@@ -15,22 +15,22 @@
     locations=[DirectiveLocation.FIELD],
     description="Only return lightweight values (e.g., scalars)",
 )
-def lightweight(field: DirectiveValue[DamnitRun | DamnitVariable]):
+def lightweight(field: DirectiveValue[DamnitRun | Cell]):
     fields = field if isinstance(field, list) else [field]
 
-    for variable in get_variables(fields):
-        if variable is not None and variable.dtype in HEAVY_DATA:
-            variable.value = None
+    for cell in get_cells(fields):
+        if cell is not None and cell.dtype in HEAVY_DATA:
+            cell.value = None
 
     # Return original field
     return field
 
 
-def get_variables(fields):
-    variables = []
+def get_cells(fields):
+    cells = []
     for field in fields:
         if isinstance(field, DamnitRun):
-            variables.extend(field._variables)
-        elif isinstance(field, DamnitVariable):
-            variables.append(field)
-    return variables
+            cells.extend(field._cells)
+        elif isinstance(field, Cell):
+            cells.append(field)
+    return cells

api/src/damnit_api/graphql/queries.py

@@ -16,7 +16,7 @@
 logger = get_logger()
 
 # Names that only `fetch_info` provides; `proposal` and `run` already come
-# from `fetch_variables`.
+# from `fetch_cells`.
 RUN_INFO_NAMES = frozenset(KNOWN_DTYPES) - {"proposal", "run"}
 
 
@@ -30,18 +30,29 @@ async def _ensure_damnit_path(info: Info, proposal: str) -> None:
     if settings.is_local:
         return
 
-    meta = await _get_proposal_meta(
-        info.context.mymdc, int(proposal), info.context.session
-    )
+    context = info.context
+    # Every aliased field in a preview request asks about the same proposal, so
+    # take the session lock once and remember the answer instead of running the
+    # same lookup concurrently on a session that cannot be shared.
+    async with context.session_lock:
+        if proposal in context.checked_proposals:
+            return
+
+        await _require_damnit_path(context, proposal)
+        context.checked_proposals.add(proposal)
+
+
+async def _require_damnit_path(context, proposal: str) -> None:
+    meta = await _get_proposal_meta(context.mymdc, int(proposal), context.session)
+    if meta.damnit_path:
+        return
+
+    logger.info("No damnit path found, updating proposal metadata")
+    meta = await _update_proposal_meta(context.mymdc, int(proposal), context.session)
     if not meta.damnit_path:
-        logger.info("No damnit path found, updating proposal metadata")
-        meta = await _update_proposal_meta(
-            info.context.mymdc, int(proposal), info.context.session
-        )
-        if not meta.damnit_path:
-            msg = "No damnit path found after updating proposal metadata."
-            # TODO: custom exceptions
-            raise ValueError(msg)
+        msg = "No damnit path found after updating proposal metadata."
+        # TODO: custom exceptions
+        raise ValueError(msg)
 
 
 def group_by_run(record):
@@ -54,7 +65,7 @@ def group_by_run(record):
                 "proposal": {"value": entry["proposal"]},
                 "run": {"value": entry["run"]},
             }
-        # Outer-join placeholder for a run with no matching variables.
+        # Outer-join placeholder for a run with no matching cells.
         if entry["name"] is None:
             continue
         grouped[key][entry["name"]] = {
@@ -66,7 +77,7 @@ def group_by_run(record):
     return list(grouped.values())
 
 
-async def fetch_variables(proposal, *, limit, offset, names=None):
+async def fetch_cells(proposal, *, limit, offset, names=None):
     table = await async_table(proposal, name="run_variables")
     if table is None:
         return []
@@ -130,15 +141,15 @@ async def fetch_variables(proposal, *, limit, offset, names=None):
         return group_by_run(result.mappings().all())  # type: ignore[assignment]
 
 
-def _selected_variable_names(info: Info) -> list[str] | None:
-    """Union the `names` arguments across every `variables` sub-selection.
+def _selected_cell_names(info: Info) -> list[str] | None:
+    """Union the `names` arguments across every `cells` sub-selection.
     Returns None if any selection omits the argument (forces a full fetch).
     """
     union = set()
     found = False
     for selected in info.selected_fields:
         for sub in selected.selections:
-            if not isinstance(sub, SelectedField) or sub.name != "variables":
+            if not isinstance(sub, SelectedField) or sub.name != "cells":
                 continue
             found = True
             arg = sub.arguments.get("names")
@@ -170,34 +181,33 @@ async def runs(
     ) -> list[DamnitRun]:
         """Return a paginated list of Damnit runs.
 
-        If the `variables` sub-selection passes a `names` argument, it is
-        pushed down to SQL so only those variables are fetched. Omitting the
-        argument returns every variable for each run.
+        If the `cells` sub-selection passes a `names` argument, it is
+        pushed down to SQL so only those cells are fetched. Omitting the
+        argument returns every cell for each run.
         """
         proposal = database.proposal
         await _ensure_damnit_path(info, proposal)
-        names = _selected_variable_names(info)
+        names = _selected_cell_names(info)
 
-        variables = await fetch_variables(
+        cells = await fetch_cells(
             proposal,
             limit=per_page,
             offset=(page - 1) * per_page,
             names=names,
         )
 
-        if not len(variables):
+        if not len(cells):
             return []
 
         if _wants_run_info(names):
             info_rows = await fetch_info(
-                proposal, runs=[v["run"]["value"] for v in variables]
+                proposal, runs=[c["run"]["value"] for c in cells]
             )
         else:
-            info_rows = [{} for _ in variables]
+            info_rows = [{} for _ in cells]
 
         return [
-            DamnitRun.from_db({**v, **i})
-            for v, i in zip(variables, info_rows, strict=True)
+            DamnitRun.from_db({**c, **i}) for c, i in zip(cells, info_rows, strict=True)
         ]
 
     @strawberry.field(permission_classes=PROPOSAL_PERMISSIONS)
@@ -234,7 +244,7 @@ async def extracted_data(
     ) -> JSON | None:  # FIX: # pyright: ignore[reportInvalidTypeForm]
         await _ensure_damnit_path(info, database.proposal)
         # TODO: Convert to Strawberry type
-        # and make it analogous to DamitVariable; e.g. `data`
+        # and make it analogous to Cell; e.g. `data`
         return get_preview_data(  # FIX: # pyright: ignore[reportReturnType]
             proposal=database.proposal,
             run=run,

api/src/damnit_api/runs/types.py

@@ -48,17 +48,17 @@ class KnownVariable:
 
 
 @strawberry.type
-class DamnitVariableError:
+class CellError:
     message: str
     cls: str
 
     @classmethod
     def from_attrs(cls, attributes):
         """Pull the error out of a `run_variables.attributes` value.
 
-        When a variable fails to execute, DAMNIT stores a JSON string like
-        ``{"error": "...", "error_cls": "..."}`` in the `attributes` column.
-        Returns a `DamnitVariableError`, or None if there is no error.
+        When a variable fails for one run, DAMNIT stores a JSON string like
+        ``{"error": "...", "error_cls": "..."}`` in that cell's `attributes`
+        column. Returns a `CellError`, or None if the cell has no error.
         """
         if not isinstance(attributes, str):
             return None
@@ -81,47 +81,47 @@ def from_attrs(cls, attributes):
 
 
 @strawberry.type
-class DamnitVariable:
+class Cell:
     name: str
     value: Any | None
     dtype: DamnitType
-    error: DamnitVariableError | None = None
+    error: CellError | None = None
 
 
 @strawberry.type
 class DamnitRun:
-    _variables: strawberry.Private[list[DamnitVariable]]
+    _cells: strawberry.Private[list[Cell]]
 
     @strawberry.field
-    def variables(self, names: list[str] | None = None) -> list[DamnitVariable]:
+    def cells(self, names: list[str] | None = None) -> list[Cell]:
         if names is None:
-            return self._variables
+            return self._cells
         requested = set(names)
-        return [v for v in self._variables if v.name in requested]
+        return [v for v in self._cells if v.name in requested]
 
     @classmethod
-    def _iter_variables(cls, record):
+    def _iter_cells(cls, record):
         for name, entry in record.items():
             if entry is None:
                 continue
             if not isinstance(entry, dict):
                 entry = {"value": entry}
             dtype = cls.get_dtype(name, entry)
             value, dtype = serialize(entry["value"], dtype=dtype)
-            error = DamnitVariableError.from_attrs(entry.get("attributes"))
-            yield DamnitVariable(name=name, value=Any(value), dtype=dtype, error=error)
+            error = CellError.from_attrs(entry.get("attributes"))
+            yield Cell(name=name, value=Any(value), dtype=dtype, error=error)
 
     @classmethod
     def from_db(cls, record):
-        return cls(_variables=list(cls._iter_variables(record)))
+        return cls(_cells=list(cls._iter_cells(record)))
 
     @classmethod
     def resolve(cls, record):
         out: dict[str, object | None] = {
             name: None for name, entry in record.items() if entry is None
         }
 
-        for v in cls._iter_variables(record):
+        for v in cls._iter_cells(record):
             if v.value is None and v.error is None:
                 out[v.name] = None
                 continue

api/src/damnit_api/shared/gql.py

@@ -1,4 +1,5 @@
-from dataclasses import dataclass
+import asyncio
+from dataclasses import dataclass, field
 
 import numpy as np
 import orjson
@@ -56,14 +57,22 @@ class Context(BaseContext):
     oauth_user: OAuthUserInfo
     session: DBSession
     _user: User | None = None
+    # One request can resolve many root fields at once (a preview aliases
+    # `extracted_data` once per run), and they all share the session above.
+    # SQLAlchemy forbids concurrent use of a single session, so the paths that
+    # memoize below take this lock before touching it. Each memoized answer then
+    # costs one serialization rather than one lookup per field.
+    session_lock: asyncio.Lock = field(default_factory=asyncio.Lock)
+    checked_proposals: set[str] = field(default_factory=set)
 
     async def get_user(self) -> User:
         """Resolve and memoize the full `User` for this request."""
-        if self._user is None:
-            self._user = await User.from_oauth_user(
-                self.mymdc, self.session, self.oauth_user
-            )
-        return self._user
+        async with self.session_lock:
+            if self._user is None:
+                self._user = await User.from_oauth_user(
+                    self.mymdc, self.session, self.oauth_user
+                )
+            return self._user
 
 
 async def get_context(  # noqa: RUF029
@@ -76,7 +85,7 @@ def get_gql_app():
     schema = Schema(
         query=Query,
         subscription=Subscription,
-        types=[run_types.DamnitVariable],
+        types=[run_types.Cell],
         directives=[gql_main.directives.lightweight],
         config=StrawberryConfig(
             auto_camel_case=False,

api/tests/graphql/conftest.py

@@ -7,7 +7,7 @@
 from damnit_api.graphql.metadata import fetch_metadata
 from damnit_api.graphql.queries import Query
 from damnit_api.graphql.subscriptions import Subscription, poll_proposal
-from damnit_api.runs.types import SCALAR_MAP, DamnitVariable
+from damnit_api.runs.types import SCALAR_MAP, Cell
 
 from .const import (
     EXAMPLE_TAGS,
@@ -105,7 +105,7 @@ def graphql_schema_no_auth(
     return strawberry.Schema(
         query=Query,
         subscription=Subscription,
-        types=[DamnitVariable],
+        types=[Cell],
         directives=[lightweight],
         config=StrawberryConfig(auto_camel_case=False, scalar_map=SCALAR_MAP),
     )

api/tests/graphql/test_models.py

@@ -9,7 +9,7 @@
     serialize,
     to_complex_string,
 )
-from damnit_api.runs.types import DamnitRun, DamnitVariableError
+from damnit_api.runs.types import CellError, DamnitRun
 from damnit_api.shared.const import DamnitType
 
 
@@ -140,14 +140,14 @@ def test_serialize_image():
 
 
 # -----------------------------------------------------------------------------
-# Test DamnitVariableError.from_attrs
+# Test CellError.from_attrs
 
 ERROR_ATTRS = {"error": "IndexError: list index out of range", "error_cls": "Foo"}
 
 
 def test_extract_error_from_json_string():
-    error = DamnitVariableError.from_attrs(json.dumps(ERROR_ATTRS))
-    assert error == DamnitVariableError(message=ERROR_ATTRS["error"], cls="Foo")
+    error = CellError.from_attrs(json.dumps(ERROR_ATTRS))
+    assert error == CellError(message=ERROR_ATTRS["error"], cls="Foo")
 
 
 @pytest.mark.parametrize(
@@ -163,7 +163,7 @@ def test_extract_error_from_json_string():
     ],
 )
 def test_extract_error_returns_none(attributes):
-    assert DamnitVariableError.from_attrs(attributes) is None
+    assert CellError.from_attrs(attributes) is None
 
 
 # -----------------------------------------------------------------------------