Build TargetDB on pfs.utils.database.db.DB

.github/workflows/lint.yml

@@ -30,8 +30,10 @@ jobs:
 
       # --output-format=github renders findings as inline PR annotations,
       # which is the part of the Codacy integration worth keeping.
+      # alembic is passed as a directory so that the versions/ exclusion in
+      # pyproject.toml applies; only the env.py scripts are actually checked.
       - name: Run ruff
-        run: uv run ruff check --output-format=github src tests
+        run: uv run ruff check --output-format=github src tests alembic
 
       - name: Run black
-        run: uv run black --check --diff src tests
+        run: uv run black --check --diff src tests alembic

.gitignore

@@ -146,3 +146,17 @@ cython_debug/
 
 # Version file generated by setuptools-scm
 src/targetdb/_version.py
+
+# Live PostgreSQL cluster created by examples/docker/docker-compose.yml.
+# Never commit this: it is the server's on-disk state, and once a role has a
+# password set, global/ holds its SCRAM verifier.
+examples/docker/db-data/
+
+# Alembic run logs kept alongside the migration directories
+alembic/*/log.alembic-*
+
+# Local scratch and tool state
+tmp/
+.pyscn/
+.pdm-python
+.python-version

.pre-commit-config.yaml

@@ -10,15 +10,16 @@ repos:
         # (e.g. models/proposal.py's `partner # noqa: F401`) that F401's
         # autofix would silently delete. Report-only also matches the
         # check-only Lint workflow.
-        # Match the Lint workflow, which checks src and tests only.
-        # alembic/ migrations are historical and deliberately unlinted.
-        files: ^(src|tests)/
+        # Match the Lint workflow, which checks src, tests and the alembic
+        # env.py scripts. The revision scripts under alembic/*/versions/ are
+        # historical and deliberately unlinted (see pyproject.toml).
+        files: ^(src|tests)/|^alembic/[^/]+/alembic/env\.py$
 
   - repo: https://github.com/psf/black
     rev: 25.12.0
     hooks:
       - id: black
-        files: ^(src|tests)/
+        files: ^(src|tests)/|^alembic/[^/]+/alembic/env\.py$
 
   - repo: https://github.com/pre-commit/pre-commit-hooks
     rev: v5.0.0

CLAUDE.md

@@ -0,0 +1,56 @@
+# CLAUDE.md
+
+This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository.
+
+See @README.md for project overview and @docs/getting_started.md for setup and usage details.
+
+## Commands
+
+```bash
+# Install (development)
+pip install -e .        # or: uv sync
+
+# Run tests
+pytest tests/
+pytest tests/test_models.py::test_relations_consistency  # single test
+
+# Lint / format
+ruff check src/
+black src/
+
+# Docs (requires pip install -e ".[doc]" or uv sync --extra doc)
+mkdocs serve
+mkdocs build
+```
+
+## CLI
+
+All CLI commands require a TOML config file (`-c dbconf.toml`). The config shape is:
+
+```toml
+[targetdb.db]
+host = "localhost"
+port = 5432
+dbname = "targetdb"
+user = "admin"
+password = "admin"   # optional; omitted from the URL when absent, so libpq uses ~/.pgpass
+dialect = "postgresql"
+```
+
+CLI flags default to `--commit False` (dry-run). Pass `--commit` explicitly to write to the database.
+
+## Non-obvious architecture
+
+- **PostgreSQL + Q3C extension required.** The Q3C extension must be installed before creating the schema (`pfs-targetdb-cli install-q3c`). It provides spatial indexing via `q3c_ang2ipix(ra, dec)` — every positional table (`target`, `fluxstd`, `sky`) has a `*_q3c_ang2ipix_idx` index.
+- **Model import order in `src/targetdb/models/__init__.py` is load-order-sensitive** due to SQLAlchemy FK relationships. New models must be imported after their FK dependencies and added to `__all__`.
+- **`input_catalog_id` is range-constrained.** The PostgreSQL `Identity` sequence starts at `10000`, max `89999`. Values up to `99999` are reserved for special use.
+- **`version` column in `fluxstd` and `sky` is a string**, not a number (e.g., `"3.3"`).
+- **All timestamps use UTC** via a custom `utcnow()` SQLAlchemy `FunctionElement` defined in `models/__init__.py`.
+- **CLI delegates entirely to `utils.py`.** `cli/cli_main.py` is only argument parsing; all business logic lives in `utils.py`.
+- **`TargetDB` is a subclass of `pfs.utils.database.db.DB`** (from the `pfs-utils` package, installed from GitHub). `DB` supplies the engine cache, `pool_pre_ping`, `COPY`-based bulk inserts and the `query_*` API. `targetdb.py` only overrides what `DB` lacks: a password/dialect in the URL, `update()`, `close()`, and the `dry_run=` keyword. Two overrides are load-bearing and easy to break:
+  - `connect()` returns `None` on purpose. `DB.connect()` hands back a pooled `Connection`, and every caller here and downstream discards it, which would leak one checked-out connection per call.
+  - `connection()` opens its transaction eagerly (`conn.begin()`) and rolls back when `_dry_run` is set. The eager begin matters because `pandas.to_sql` starts _and commits_ a transaction of its own when handed a connection that is not already in one, which would defeat the rollback. Every dry-run path funnels through here. `tests/integration/test_dry_run.py` guards this.
+- **`TargetDB`'s connection defaults live in `DEFAULT_HOST`/`DEFAULT_USER`/`DEFAULT_DBNAME`/`DEFAULT_PORT` class attributes**, the same convention `OpDB`/`QaDB` use in `pfs.utils.database.db`. `DB.__init__` resolves any argument left `None` from `type(self).DEFAULT_*`, so every parameter in `TargetDB.__init__` must default to `None`, never to a literal — a non-`None` default in the signature shadows the class attribute and silently breaks the inherited `set_default_connection()` classmethod. `DEFAULT_USER` is `obsproc`, a read-only account, on purpose: these defaults only apply to a bare `TargetDB()`, since every write path (the CLI, `utils.add_database_rows`, …) splats a full `[targetdb.db]` table with an explicit `user`, so a least-privilege default costs nothing and makes an accidental write to production through a bare `TargetDB()` impossible.
+- **psycopg3 only.** `dialect = "postgresql"` in a config file is rewritten to `postgresql+psycopg` by `normalize_drivername()` in `utils.py`, the single choke point that every code path (`get_url_object`, `TargetDB.__init__`) passes through. psycopg2 is not a dependency.
+- **Inserts drop DataFrame columns the target table does not have** (`TargetDB._drop_unknown_columns`). `add_backref_values()` resolves foreign keys by merging whole reference tables in, leaving columns like `partner_name` behind; the old ORM path ignored unmapped keys and `COPY` does not.
+- **Alembic has separate directories per deployment target** under `alembic/` (`local_test/`, `pfsa-db01-gb/`, `pfsa-db01-gb-dev/`). Each has its own `alembic.ini`. Run migrations from within the appropriate subdirectory. `env.py` builds the URL from the TOML file named by the `TARGETDB_CONF` environment variable, falling back to `sqlalchemy.url` in `alembic.ini` when it is unset — so credentials live in one place: `TARGETDB_CONF=dbconf.toml alembic upgrade head`. Do not use `config.set_main_option()` for the URL: ConfigParser reads `%` as an interpolation marker and would mangle passwords containing it. See `alembic/README.md` for the workflow and known gotchas (q3c expression indexes are invisible to autogenerate; `local_test/`'s revision chain is broken).

README.md

@@ -19,29 +19,28 @@ The Q3C extension is required for the database. You can install it by the follow
 ### Python environment
 
 Python and the following packages as well as their dependencies will be used by installing `targetdb`.
-Package versions shown here are those used for the development (as of April 2025).
+Package versions shown here are those used for the development (as of May 2026).
 Newer (and somewhat older) versions should also work.
 
-| Package                                                                | Version |
-|------------------------------------------------------------------------|--------:|
-| [Python](https://www.python.org/)                                      |  3.11.x |
-| [SQLAlchemy](https://www.sqlalchemy.org/)                              |   2.0.x |
-| [pandas](https://pandas.pydata.org/)                                   |   2.2.3 |
-| [NumPy](https://numpy.org)                                             |  1.26.4 |
-| [Astropy](https://www.astropy.org/)                                    |   7.0.1 |
-| [loguru](https://loguru.readthedocs.io/)                               |   0.7.3 |
-| [SQLAlchemy-Utils](https://sqlalchemy-utils.readthedocs.io/en/latest/) |  0.41.2 |
-| [tabulate](https://pypi.org/project/tabulate/)                         |   0.9.0 |
-| [alembic](https://alembic.sqlalchemy.org/en/latest/)                   |  1.13.1 |
-| [pyarrow](https://arrow.apache.org/docs/python/)                       |  15.0.2 |
-| [Typer](https://typer.tiangolo.com/)                                   |  0.15.2 |
-| [openpyxl](https://openpyxl.readthedocs.io/en/stable/)                 |   3.1.2 |
-
-If you are using Python 3.10 or earlier, you may need to install [tomli](https://github.com/hukkin/tomli) package.
-
-| Package                                  | Version |
-|------------------------------------------|--------:|
-| [tomli](https://github.com/hukkin/tomli) |   2.0.1 |
+| Package                                                                |     Version |
+|------------------------------------------------------------------------|------------:|
+| [Python](https://www.python.org/)                                      |      3.12.x |
+| [SQLAlchemy](https://www.sqlalchemy.org/)                              |       2.0.x |
+| [pandas](https://pandas.pydata.org/)                                   |       2.3.3 |
+| [NumPy](https://numpy.org)                                             |       2.4.0 |
+| [Astropy](https://www.astropy.org/)                                    |       7.2.0 |
+| [loguru](https://loguru.readthedocs.io/)                               |       0.7.3 |
+| [pfs-utils](https://github.com/Subaru-PFS/pfs_utils)                   | 7.2026.3100 |
+| [psycopg](https://www.psycopg.org/)                                    |       3.3.4 |
+| [SQLAlchemy-Utils](https://sqlalchemy-utils.readthedocs.io/en/latest/) |      0.42.1 |
+| [tabulate](https://pypi.org/project/tabulate/)                         |       0.9.0 |
+| [alembic](https://alembic.sqlalchemy.org/en/latest/)                   |      1.18.0 |
+| [pyarrow](https://arrow.apache.org/docs/python/)                       |      22.0.0 |
+| [Typer](https://typer.tiangolo.com/)                                   |      0.21.1 |
+| [openpyxl](https://openpyxl.readthedocs.io/en/stable/)                 |       3.1.5 |
+
+`targetdb.TargetDB` subclasses `pfs.utils.database.db.DB`, so `pfs-utils` is installed
+straight from GitHub and a `.git` directory plus network access are needed at install time.
 
 For building the documentation, the following packages are required.
 

alembic/README.md

@@ -0,0 +1,143 @@
+# Alembic migrations for targetdb
+
+Schema changes to `targetdb` are applied with
+[Alembic](https://alembic.sqlalchemy.org/en/latest/tutorial.html). The models in
+`src/targetdb/models/` are the source of truth; the revision scripts here record
+how each deployed database was brought to match them.
+
+## Layout
+
+Each deployment target has its own directory, with its own `alembic.ini` and its
+own independent revision history. **Run alembic from inside the directory for
+the database you are migrating** — the histories are not interchangeable.
+
+| Directory           | Database                                        |
+| ------------------- | ----------------------------------------------- |
+| `local_test/`       | A local database used for trying migrations out |
+| `pfsa-db01-gb/`     | Production                                      |
+| `pfsa-db01-gb-dev/` | Development                                     |
+
+Directory names are historical and no longer match the current hostnames:
+`pfsa-db01-gb` and `pfsa-db` resolve to different machines (`.102` and
+`.110` respectively), and as of 2026-08 the production `targetdb` — the one
+`TargetDB`'s own `DEFAULT_HOST`/`DEFAULT_PORT` point at — is reachable at
+`pfsa-db:5433`, not `pfsa-db01-gb:5433`. Confirm which host actually holds
+the database you intend to migrate before running `alembic upgrade` from
+either directory.
+
+The revision scripts under `*/alembic/versions/` are historical records. They
+are deliberately excluded from `ruff` and `black` (see `pyproject.toml`) and
+should not be reformatted or "cleaned up" — only the `env.py` scripts are
+linted. Once a revision has been applied to a deployed database, edit it only
+to fix something that is actually broken.
+
+## Configuring the connection
+
+Set `TARGETDB_CONF` to a targetdb TOML configuration file — the same one the
+CLI takes with `-c` — and `env.py` builds the connection URL from it:
+
+```bash
+cd alembic/pfsa-db01-gb
+TARGETDB_CONF=~/database_configs/config_targetdb.toml alembic upgrade head
+```
+
+This keeps the credentials in one place rather than duplicating them into every
+`alembic.ini`, and the psycopg3 driver is selected automatically. If
+`TARGETDB_CONF` is unset, `env.py` falls back to the `sqlalchemy.url` entry in
+`alembic.ini`, which is a credentials-free placeholder.
+
+Passwords may be left out of the TOML entirely, in which case libpq resolves
+them from `PGPASSWORD` or `~/.pgpass`. See `docs/getting_started.md`.
+
+## Workflow
+
+1. **Change the models** under `src/targetdb/models/`.
+
+2. **Generate a revision.** Autogenerate compares the models against the live
+   database, so this needs a database that is currently at `head`:
+
+   ```bash
+   TARGETDB_CONF=<config.toml> alembic revision --autogenerate -m "Add columns"
+   ```
+
+3. **Read the generated file** in `versions/` before running it. Autogenerate is
+   a starting point, not an answer — see the gotchas below for what it gets
+   wrong on this schema.
+
+4. **Apply it.**
+
+   ```bash
+   TARGETDB_CONF=<config.toml> alembic upgrade head
+   ```
+
+   Check where you are at any time with `alembic current`, and what exists with
+   `alembic history`.
+
+Try a migration against `local_test/` (or a throwaway container — see
+`tests/docker/`) before running it on a deployed database.
+
+## Gotchas
+
+### A unique constraint cannot be added while duplicates exist
+
+`CREATE UNIQUE INDEX` fails outright if the table already violates the
+constraint, and the migration aborts partway:
+
+```text
+UniqueViolation: could not create unique index "uq_sky_obj_id_input_catalog_id_version"
+DETAIL:  Key (obj_id, input_catalog_id, version)=(44332, 1002, 20220915) is duplicated.
+```
+
+Count the offending rows first, and decide what to do with them before writing
+the migration:
+
+```sql
+-- How many groups are duplicated, and by how much?
+SELECT ct, count(*) AS n_groups
+FROM (
+    SELECT obj_id, input_catalog_id, version, count(*) AS ct
+    FROM sky
+    GROUP BY obj_id, input_catalog_id, version
+    HAVING count(*) > 1
+) sub
+GROUP BY 1 ORDER BY 1;
+
+-- Which rows are they?
+SELECT obj_id, ra, dec, input_catalog_id, version, count(*)
+FROM sky
+GROUP BY obj_id, ra, dec, input_catalog_id, version
+HAVING count(*) > 1;
+```
+
+Substitute the table and the intended constraint columns. `sky` has run to
+billions of rows per `version`, so on it and `fluxstd` expect these queries to
+take a while, and restrict them by `version` where you can.
+
+### Autogenerate does not see the q3c indexes
+
+The positional tables are indexed on the expression `q3c_ang2ipix(ra, dec)`, and
+SQLAlchemy cannot reflect expression-based indexes:
+
+```text
+SAWarning: Skipped unsupported reflection of expression-based index sky_q3c_ang2ipix_idx
+```
+
+The warning is harmless, but it means autogenerate will neither create nor
+notice a change to those indexes. Write that DDL by hand.
+
+Identity/serial sequences are likewise reported as skipped
+(`Detected sequence named 'fluxstd_fluxstd_id_seq' ... assuming SERIAL and
+omitting`); this is normal and needs nothing.
+
+### `%` in a password
+
+`env.py` builds the engine directly rather than going through
+`config.set_main_option()`, because ConfigParser reads `%` as an interpolation
+marker and would corrupt a password containing one. Keep it that way.
+
+### `local_test/` has a broken revision chain
+
+`80f8276e2ee7` names a `down_revision` (`ecfad41204d1`) that is not present in
+the repository, so any command that walks the full revision map — `alembic
+current` and `alembic history` included — raises `KeyError`. This predates the
+move to `TARGETDB_CONF`; connecting to the database itself works.

alembic/local_test/alembic.ini

@@ -0,0 +1,77 @@
+# A generic, single database configuration.
+
+[alembic]
+# path to migration scripts
+script_location = alembic
+
+# template used to generate migration files
+file_template = %%(year)d%%(month).2d%%(day).2d-%%(hour).2d%%(minute).2d%%(second).2d_%%(rev)s_%%(slug)s
+
+# sys.path path, will be prepended to sys.path if present.
+prepend_sys_path = .
+
+# timezone used for the date in the migration file and its filename.
+# UTC matches the rest of targetdb and keeps filenames identical whichever
+# machine generates them. Resolved via dateutil, which pandas already pulls in.
+timezone = UTC
+
+# max length of characters to apply to the "slug" field
+truncate_slug_length = 40
+
+# run env.py during the 'revision' command even without --autogenerate
+revision_environment = false
+
+# allow .pyc/.pyo files without a source .py to be detected as revisions
+sourceless = false
+
+# version path separator; "os" uses os.pathsep.
+version_path_separator = os
+
+# the output encoding used when revision files are written from script.py.mako
+output_encoding = utf-8
+
+# Fallback only: env.py builds the URL from the TOML file named by the
+# TARGETDB_CONF environment variable when it is set. Placeholder -- do not
+# commit real credentials.
+sqlalchemy.url = postgresql://username:password@localhost:5432/targetdb
+
+
+[post_write_hooks]
+# Deliberately empty: alembic/ is excluded from black and ruff in
+# pyproject.toml, so generated revisions are left as alembic writes them.
+
+
+# Logging configuration
+[loggers]
+keys = root,sqlalchemy,alembic
+
+[handlers]
+keys = console
+
+[formatters]
+keys = generic
+
+[logger_root]
+level = WARN
+handlers = console
+qualname =
+
+[logger_sqlalchemy]
+level = WARN
+handlers =
+qualname = sqlalchemy.engine
+
+[logger_alembic]
+level = INFO
+handlers =
+qualname = alembic
+
+[handler_console]
+class = StreamHandler
+args = (sys.stderr,)
+level = NOTSET
+formatter = generic
+
+[formatter_generic]
+format = %(levelname)-5.5s [%(name)s] %(message)s
+datefmt = %H:%M:%S