Fix the concat --merge hang, and close the gap that hid it

.dockerignore

@@ -0,0 +1,18 @@
+# Keep the build context to what `uv sync --locked` actually needs. In
+# particular .venv/ would otherwise ship a host-platform virtualenv into the
+# image, and .claude/worktrees/ holds full copies of this repo.
+#
+# Do NOT add README.md or LICENSE here: pyproject.toml references them via
+# `readme` and `license-files`, so the build fails without them.
+.venv/
+.git/
+.claude/
+.pytest_cache/
+__pycache__/
+*.py[cod]
+.coverage
+coverage.xml
+htmlcov/
+build/
+dist/
+*.egg-info/

.github/workflows/benchmark.yml

@@ -0,0 +1,137 @@
+# Comparative benchmark: adata-cli against anndata and scanpy.
+#
+# Report-only, by design. Wall time on a shared runner is too noisy to gate a
+# release on, and a benchmark that can block a publish stops being run. What
+# *does* gate merges is tests/test_performance.py, which counts operations
+# rather than seconds and lives in the ordinary test job.
+#
+# Its own file rather than a job in publish.yml: at the `ci` tier this takes
+# the better part of an hour, and hanging that off the release graph would
+# either delay the PyPI publish or paint the release run red for a report.
+name: Benchmark
+
+on:
+  push:
+    tags: ["*"]
+  workflow_dispatch:
+    inputs:
+      tier:
+        description: Input size
+        type: choice
+        options: [smoke, ci, large]
+        default: ci
+      publish:
+        description: Commit the results to docs/ on main
+        type: boolean
+        default: false
+
+concurrency:
+  group: benchmark-${{ github.ref }}
+  cancel-in-progress: true
+
+jobs:
+  benchmark:
+    name: adata-cli vs anndata/scanpy
+    runs-on: ubuntu-latest
+    timeout-minutes: 90
+    # Job level, not step level: a report must never mark a release red.
+    continue-on-error: true
+    permissions:
+      contents: write
+    steps:
+      - uses: actions/checkout@v4
+        with:
+          fetch-depth: 0
+
+      - uses: astral-sh/setup-uv@v3
+        with:
+          enable-cache: true
+
+      # The checked-out source, not the published wheel. Benchmarking the
+      # wheel would mean waiting on publish-pypi and then on index
+      # propagation, for a measurement that would come out the same.
+      - name: Install adata-cli
+        run: uv sync --extra dev --frozen
+
+      - name: Show free space
+        run: df -h /
+
+      - name: Run
+        run: |
+          uv run python -m benchmarks.run \
+            --tier "${{ inputs.tier || 'ci' }}" \
+            --out results.json \
+            --work "$RUNNER_TEMP/bench"
+
+      - name: Render
+        run: |
+          uv run python -m benchmarks.report results.json --out summary.md
+          cat summary.md >> "$GITHUB_STEP_SUMMARY"
+
+      - uses: actions/upload-artifact@v4
+        if: always()
+        with:
+          name: benchmark-${{ github.ref_name }}
+          path: |
+            results.json
+            summary.md
+          retention-days: 90
+
+      # docs/ is the durable home: artifacts expire, and one absolute number
+      # with nothing to compare it against says very little. The page is
+      # already served by Pages from docs/, so this needs no extra machinery.
+      - name: Publish to docs
+        if: github.ref_type == 'tag' || inputs.publish
+        run: |
+          git config user.name  "github-actions[bot]"
+          git config user.email "41898282+github-actions[bot]@users.noreply.github.com"
+          # A tag build is on a detached HEAD; the page belongs on main.
+          git fetch origin main
+          git checkout -B main origin/main
+          uv run python -m benchmarks.report results.json --docs docs --publish
+          git add docs/BENCHMARKS.md docs/benchmarks
+          if git diff --cached --quiet; then
+            echo "nothing to publish"; exit 0
+          fi
+          git commit -m "Benchmark results for ${{ github.ref_name }} [skip ci]"
+          # Another job may have landed on main in the meantime.
+          for attempt in 1 2 3; do
+            if git push origin main; then exit 0; fi
+            git pull --rebase origin main
+          done
+          echo "could not push benchmark results"; exit 1
+
+      # Best effort, and last: releases here are cut by hand, so this job may
+      # well run before one exists. The docs page is authoritative either way.
+      - name: Append to the release notes
+        if: github.ref_type == 'tag'
+        continue-on-error: true
+        env:
+          GH_TOKEN: ${{ github.token }}
+        run: |
+          tag="${{ github.ref_name }}"
+          gh release view "$tag" >/dev/null 2>&1 || {
+            echo "no release for $tag yet; skipping"; exit 0
+          }
+          gh release view "$tag" --json body -q .body > body.md
+          # Idempotent: replace any block this job wrote before.
+          python - <<'PY'
+          import pathlib, re
+          body = pathlib.Path("body.md").read_text()
+          summary = pathlib.Path("summary.md").read_text()
+          block = (
+              "<!-- benchmark:start -->\n\n"
+              "<details><summary>Benchmark vs anndata/scanpy</summary>\n\n"
+              f"{summary}\n</details>\n\n<!-- benchmark:end -->"
+          )
+          pattern = re.compile(
+              r"<!-- benchmark:start -->.*?<!-- benchmark:end -->", re.S
+          )
+          body = (
+              pattern.sub(block, body)
+              if pattern.search(body)
+              else body.rstrip() + "\n\n" + block + "\n"
+          )
+          pathlib.Path("body.md").write_text(body)
+          PY
+          gh release edit "$tag" --notes-file body.md

.gitignore

@@ -10,3 +10,9 @@ htmlcov/
 .pytest_cache/
 pytest-results*.xml
 compat-results.xml
+
+# Benchmark working directory and results -- the defaults of
+# `python -m benchmarks.run`, both written into the repo root.
+.bench/
+results.json
+summary.md

CHANGELOG.md

@@ -3,6 +3,117 @@
 Notable changes to `adata-cli`. Versions are `MAJOR.MINOR.PATCH`; tags carry no
 `v` prefix.
 
+## Unreleased
+
+### Fixed
+
+- **`concat --merge` never finished on a real store.** Aligning a var column
+  onto the target index re-read the whole column from disk once per target
+  variable, so the cost was quadratic: at 36,601 variables a merge that should
+  take a fraction of a second ran for hours at 100% CPU with the output file
+  never growing past its header. Reported against 0.5.1 (REQ-71798), where 12
+  of 13 pipeline tasks had to be killed after 98 minutes. The column is now
+  read once per input, and `--merge first` / `--merge only`, which decide on
+  presence alone, read no column values at all.
+- **`concat` was quadratic in the number of categories** in an obs column.
+  Category merging probed a list rather than a dict: 2,096,128 string
+  comparisons to union 1,024 categories, and around 5e9 for a 100k-category
+  column. Found by the new guards.
+- **`split --by` was quadratic**, O(n_rows x n_groups). `group_indices` grouped
+  rows with `np.nonzero(values == label)` inside a loop over distinct labels,
+  rescanning each chunk once per label: at 4,096 rows, 16,384 elements scanned
+  for 4 groups and 1,048,576 for 256. A million cells split by a thousand
+  samples is ~10^9 comparisons. One `np.unique` pass per chunk makes it flat
+  in the group count. Found by the new guards; order of first appearance,
+  which names the output files, is unchanged.
+- **`concat` built a Python object per row** for nullable and string obs
+  columns, then walked the list twice more. Filling a typed buffer by slice
+  removes three full passes over every such column.
+
+### Added
+
+- **Complexity guards in the test suite** (`tests/test_performance.py`).
+  Cost regressions now fail at merge time. They count operations rather than
+  seconds -- h5py and zarr reads, Zarr store traffic, Python allocation and
+  executed lines -- and assert that successive increments grow no faster than
+  linearly, so nothing here can fail because a CI runner was busy. See
+  [docs/TESTING.md](docs/TESTING.md#performance). Every subcommand is covered:
+  `ls`, `view`, `create`, all five `export` and all five `import` variants,
+  `split` on both axes, and the `concat` options nothing else reached.
+  Two claims are now enforced rather than described -- `view` and `ls` read
+  **zero** data elements at any store size, and streaming stays far below the
+  input curve at a fixed `--chunk`.
+- **Copying variable-length strings ignored its own read budget.** The width
+  of a vlen element was assumed to be 64 bytes, because h5py reports the
+  itemsize of a pointer, so the step was the same 524,288 elements whatever
+  the data held: 2 GiB per read at 4 KiB elements against a stated 32 MiB
+  budget, and for any array shorter than that step, the whole array in one
+  go. Copying 200,000 strings of 4 KiB peaked at 827 MB. The width is now
+  sampled from the first 256 elements. Reported by an automated review on
+  PR #14 and confirmed by measurement; `uns` can hold arbitrary text, so this
+  was not a width the layer could assume.
+- **Peak RSS in the benchmark was floored by the runner's own memory on Linux.**
+  A forked child inherits its parent's resident pages and `execve` folds that
+  into the `maxrss` the kernel reports, so every contender would have measured
+  at least what `benchmarks/run.py` used to build the fixtures — around
+  200 MB — and the tables would have read "everything costs about the same".
+  Commands are now forked from a small shim: with a 330 MB parent, a no-op
+  child goes from 326 MB to 8 MB. Caught by `test_benchmark_harness.py`, which
+  exists for exactly this. The published figures were measured on macOS, which
+  resets the high-water mark at exec, and are unchanged.
+- **A comparative benchmark** (`benchmarks/`), run on every tag against
+  anndata and against scanpy where scanpy has a real equivalent. Reports peak
+  RSS, wall time and output size; publishes to
+  [docs/BENCHMARKS.md](docs/BENCHMARKS.md) and the release notes. Report-only
+  -- it never fails a build. Fifteen cases, covering every command with a real
+  baseline, including `h5ls -r` for `ls` and the rows where adata-cli is the
+  slower of the two.
+- **`--merge drop` and `--uns-merge drop` are accepted.** `drop` was already
+  the documented default behaviour but was rejected as a value, so a config
+  could not state it explicitly.
+
+## 0.5.1
+
+Makes the container image usable from Nextflow, and stops `copy_dataset`
+reading one row at a time from row-chunked stores.
+
+### Fixed
+
+- **Copying a row-chunked store was dominated by read latency.** The read step
+  was the source's chunk height verbatim, so a store chunked `(1, n_cols)` was
+  copied one row per read. On a network filesystem (Lustre, NFS) each read is a
+  round-trip, so a million-row copy spent nearly all of its time waiting. Reads
+  are now sized to a 32 MiB budget, rounded down to a whole number of source
+  chunks. A `(1_000_000, 30_000)` float32 store chunked `(1, 30_000)` goes from
+  1 row per read to 279.
+- Read sizing no longer trusts `itemsize` for variable-length strings. h5py
+  reports 8 there because the value is a pointer, which overestimated the row
+  count by an order of magnitude and broke the memory bound.
+
+### Container
+
+- **The image could not be used from a Nextflow process.** Nextflow requires
+  `/bin/bash` to be the container entrypoint, so `ENTRYPOINT ["adata"]` made
+  every Docker- and Podman-backed task fail with `No such command
+  '/bin/bash'`. Apptainer was unaffected, as `singularity exec` ignores the
+  entrypoint.
+- **Task metrics were silently lost.** `procps` is absent from the base image,
+  so Nextflow could not run `ps` to collect them. The required tool set
+  (`bash`, `ps`, `awk`, `date`, `grep`, `sed`, `tail`, `tee`) is now installed
+  and asserted at build time.
+- `PYTHONNOUSERSITE` is set, so a bind-mounted `$HOME` under Apptainer can no
+  longer shadow the image's virtualenv with the user's `~/.local` packages.
+- `XDG_CACHE_HOME` points at `/tmp`, so the image tolerates being run under an
+  arbitrary UID with no writable `$HOME`.
+- Added a `.dockerignore`. Local builds were copying the host's `.venv`,
+  `.git` and `.pytest_cache` into the image.
+
+### Changed
+
+- **The image no longer sets an entrypoint, so the command must be named
+  explicitly:** `docker run IMAGE adata view file.h5ad`, where `docker run
+  IMAGE view file.h5ad` previously worked.
+
 ## 0.5.0
 
 Renamed from `h5ad` to `adata-cli`, restored compatibility with current

Dockerfile

@@ -1,24 +1,33 @@
 # Base image: Python 3.12 + uv preinstalled (Debian slim)
 FROM ghcr.io/astral-sh/uv:python3.12-bookworm-slim
 
-ENV UV_NO_DEV=1
+# PYTHONNOUSERSITE: Apptainer bind-mounts the host $HOME by default, so a user's
+#   ~/.local/lib/python3.12/site-packages would otherwise shadow this venv.
+# XDG_CACHE_HOME: Nextflow is commonly configured with `-u $(id -u):$(id -g)`,
+#   which leaves the container with no writable $HOME.
+# UV_COMPILE_BYTECODE: bake .pyc at build time, so nothing writes to a
+#   read-only rootfs on first import.
+ENV UV_NO_DEV=1 \
+    UV_COMPILE_BYTECODE=1 \
+    PYTHONNOUSERSITE=1 \
+    PYTHONDONTWRITEBYTECODE=1 \
+    PYTHONUNBUFFERED=1 \
+    XDG_CACHE_HOME=/tmp/.cache
 
-WORKDIR /cli
-
-# Copy the project files (from the GitHub Actions checkout context)
-COPY . .
-
-# --locked asserts that uv.lock is in sync with pyproject.toml, so an image
-# can never be built from a lockfile that drifted.
-RUN uv sync --locked
+# procps supplies `ps`, which Nextflow needs to collect per-task metrics.
+# curl, unzip and ca-certificates fetch duckdb below, and are left in place
+# rather than purged: pipeline scripts routinely reach for curl, and TLS
+# roots are worth having in any container that may touch the network.
+RUN apt-get update \
+    && apt-get install -y --no-install-recommends \
+         ca-certificates curl procps mawk unzip \
+    && rm -rf /var/lib/apt/lists/*
 
 # duckdb, for the filtering workflows in the docs: export obs to CSV, query it,
 # feed the names back to `adata subset --obs`. A single static binary, so it
 # needs no venv and cannot conflict with the project's dependencies.
 ARG DUCKDB_VERSION=v1.1.3
-RUN apt-get update \
-    && apt-get install -y --no-install-recommends ca-certificates curl unzip \
-    && ARCH="$(dpkg --print-architecture)" \
+RUN ARCH="$(dpkg --print-architecture)" \
     && case "$ARCH" in \
          amd64) DUCKDB_ARCH=amd64 ;; \
          arm64) DUCKDB_ARCH=aarch64 ;; \
@@ -28,13 +37,43 @@ RUN apt-get update \
          "https://github.com/duckdb/duckdb/releases/download/${DUCKDB_VERSION}/duckdb_cli-linux-${DUCKDB_ARCH}.zip" \
     && unzip -q /tmp/duckdb.zip -d /usr/local/bin \
     && chmod +x /usr/local/bin/duckdb \
-    && rm /tmp/duckdb.zip \
-    && apt-get purge -y curl unzip \
-    && apt-get autoremove -y \
-    && rm -rf /var/lib/apt/lists/*
+    && rm /tmp/duckdb.zip
+
+# Fail the build, rather than every Nextflow task, if the base image ever drops
+# one of the tools Nextflow requires in a task container.
+RUN set -eu; for t in bash ps awk date grep sed tail tee; do \
+      command -v "$t" >/dev/null || { echo "missing required tool: $t" >&2; exit 1; }; \
+    done
+
+WORKDIR /cli
+
+# Copy the project files (from the GitHub Actions checkout context)
+COPY . .
+
+# --locked asserts that uv.lock is in sync with pyproject.toml, so an image
+# can never be built from a lockfile that drifted.
+#
+# uv honours XDG_CACHE_HOME, so the sync leaves a root-owned package cache at
+# /tmp/.cache -- which made the variable self-defeating, as a task running
+# under an arbitrary UID then could not write to the very path it advertises.
+# Clear it and leave an empty world-writable directory behind. Done in this
+# same layer because a later `rm` would mask the files without reclaiming
+# them; that reclaims about 9 MB, the cache being mostly hardlinks into the
+# venv rather than separate copies.
+RUN uv sync --locked \
+    && rm -rf /tmp/.cache /tmp/uv-*.lock \
+    && mkdir -p /tmp/.cache \
+    && chmod 1777 /tmp/.cache
 
 # Put the project venv on PATH so `adata` is directly runnable
 ENV PATH="/cli/.venv/bin:${PATH}"
 
-ENTRYPOINT ["adata"]
-CMD ["--help"]
+# No ENTRYPOINT on purpose: Nextflow requires /bin/bash to be the container
+# entrypoint, so the image must not set one of its own. This is why invocations
+# spell out the command: `docker run IMAGE adata view file.h5ad`.
+#
+# Deliberately NOT set here: OMP_NUM_THREADS / OPENBLAS_NUM_THREADS. NumPy's
+# BLAS sizes its thread pool to the whole host, which oversubscribes a shared
+# LSF node. This workload is streaming I/O, so capping it would cost nothing --
+# but it belongs in the pipeline's `env` scope, not baked into the image.
+CMD ["adata", "--help"]