ci: pytest + alembic check gates; reconcile index drift #21

Merged

john merged 1 commit from fix/ci-pytest-and-index-drift into dev

2026-08-03 23:57:55 +00:00

john commented

2026-08-03 23:30:37 +00:00

Owner

Two hygiene fixes. No behaviour change to the application: the model edits emit no DDL and no migration is added.

Deliverable 2 — the two index drifts

uv run alembic check failed on two indexes that exist in the database and in no model. Both were created by this repo's own migrations, which makes the migrations the truthful side and the models the drifted one. Aligned the models; no migration written, alembic heads unchanged at c5e19a7d3f60.

Index	Declared by	What it is	Model was
`billing_profile_invoice_prefix_key`	`f64a7ec6edcf`	`UNIQUE (invoice_prefix) WHERE deleted_at IS NULL`	silent — no `__table_args__` at all
`ix_invoice_status`	`001_initial_invoice_tables`	plain btree on `invoice(status)`	`status` had no `index=True`

Verified against production, not inferred from it. contabo-sg, dokku-postgres-haskos-db, read-only:

 billing_profile_invoice_prefix_key | CREATE UNIQUE INDEX ... USING btree (invoice_prefix) WHERE (deleted_at IS NULL)
 ix_invoice_status                  | CREATE INDEX ... USING btree (status)

Both live, both matching the new declarations. The compiled model DDL (CreateIndex(...).compile(postgresql)) was diffed against pg_get_indexdef on a scratch database and matches character for character modulo Postgres's canonical formatting (public. qualification, USING btree, parenthesised predicate).

That last check is worth more than alembic check passing, and is why it was done separately: alembic does not diff a partial index's WHERE clause. A model declaring billing_profile_invoice_prefix_key without deleted_at IS NULL would also have satisfied the gate — while silently disagreeing with production about whether retiring a billing profile releases its invoice prefix for reuse. The predicate is load-bearing and is now stated in the model with the reasoning attached.

Deliverable 1 — the CI jobs

New Tests job, runs-on: ci, gated on detect-changes.outputs.has_code like its siblings. It runs the full suite — 180 tests, including all 22 DB-backed ones — against a pgvector/pgvector:pg16 service container on --tmpfs, mirroring the pattern already proven in haskos-engineering.

Steps: uv sync → alembic upgrade head → alembic check → pytest tests/ -q -rs.

Three decisions worth reviewing:

alembic check lives here rather than in its own job because it needs a migrated database, which this job already has. It is only safe at all because alembic/env.py filters autogenerate to OWNED_TABLES — unfiltered, it proposes dropping 52 tables belonging to five other applications.
A skip fails the job. The suite's only skip mechanism is skipif(not HASKOS_TEST_DATABASE_URL) on the DB tier, so a skip in CI means that tier vanished rather than a test being excused. The Migrations step cannot catch this — it reads HASKOS_DATABASE_URL, a different variable, so a typo in the test URL leaves the schema built, migrations green, and 22 tests quietly gone. That is the shape haskos-engineering shipped (432 passed, 189 skipped in CI against 618 passed locally), and it survived because the number on the left is the one people read. No || true anywhere; verified by running the step's exact shell with the variable unset — it exits 1.
deploy now needs: [test, docker-build]. With needs: [docker-build] alone, a red suite would have deployed to prod beside a green build and the gate would have been decorative. needs: treats skipped as not-satisfied, so a docs-only push still skips the deploy exactly as it does today.

HASKOS_DEBUG: "true" is set on this job and is deliberately not set on the smoke test below it. The smoke test exists to boot the way production boots, so it must not skip the secret validation production runs. This job exercises the domain layer against a scratch database and has no production shape to reproduce — haskos_kernel.config would otherwise reject the placeholder secrets at import and never reach a test.

Spec Drift Callouts

Test count is 180, not ~265. 158 dependency-free + 22 DB-backed, across three files.
UV_INDEX_HASKYTECH_PASSWORD alone is not enough. The brief says to mirror the Docker Build job, which passes only the password — that works there because the Dockerfile supplies UV_INDEX_HASKYTECH_USERNAME=__token__. A bare uv sync on the runner has no Dockerfile behind it, so this job sets both. Copying Docker Build literally would have failed to resolve haskytech-haskos-kernel.
No fallback tier was needed. The brief offered a dependency-free-only fallback if the service container proved fragile. It did not: the full suite including all 22 DB tests passes, and is re-runnable against the same database (the tests delete exactly the rows they wrote).
The skip guard is an addition, not in the brief. Judged in scope given the explicit "no || true" instruction — a suite that silently skips its DB tier is a suppressed gate wearing a passing summary line.
deploy needing test is an addition, not in the brief. Same reasoning: without it the new gate does not gate the deploy.

Out of scope, flagged not fixed

Dockerfile HEALTHCHECK uses curl -f with --start-period=30s. Org convention is wget -qO /dev/null and 120s+ for a Python app. Untouched — unrelated to either deliverable, and changing the health probe of a deployed app is not a hygiene edit.
The new Tests job will not be a required status context until branch protection is updated for this repo; it blocks nothing on its own.
~/bin/haskos-db is broken — it discovers databases from the Coolify API and the fleet has moved to Dokku, so it dies in json.load on an empty response. The prod verification above went via ssh contabo-sg + docker exec instead.

Validation

Check	Result
`ruff check app/ tests/ scripts/`	pass
`ruff format --check app/ tests/ scripts/`	46 files already formatted
`uv run pytest tests/ -q`	180 passed, 0 skipped
`uv run alembic check`	`No new upgrade operations detected.`
`uv run alembic heads`	`c5e19a7d3f60 (head)` — one head, unchanged
Migrations added	none
Skip guard fires when DB tier absent	verified, exits 1
Model DDL vs `pg_get_indexdef`	equivalent, both indexes
Prod index existence	confirmed on `contabo-sg`
`ci.yml` parses	`yaml.safe_load`, jobs and `needs` asserted

Run against local PostgreSQL 17, scratch database haskos_fin_ci.

Do not merge without review — merging auto-deploys to production.

Two hygiene fixes. No behaviour change to the application: the model edits emit no DDL and no migration is added. ## Deliverable 2 — the two index drifts `uv run alembic check` failed on two indexes that exist in the database and in no model. Both were created by **this repo's own migrations**, which makes the migrations the truthful side and the models the drifted one. Aligned the models; **no migration written, `alembic heads` unchanged at `c5e19a7d3f60`**. | Index | Declared by | What it is | Model was | |---|---|---|---| | `billing_profile_invoice_prefix_key` | `f64a7ec6edcf` | `UNIQUE (invoice_prefix) WHERE deleted_at IS NULL` | silent — no `__table_args__` at all | | `ix_invoice_status` | `001_initial_invoice_tables` | plain btree on `invoice(status)` | `status` had no `index=True` | **Verified against production, not inferred from it.** `contabo-sg`, `dokku-postgres-haskos-db`, read-only: ``` billing_profile_invoice_prefix_key | CREATE UNIQUE INDEX ... USING btree (invoice_prefix) WHERE (deleted_at IS NULL) ix_invoice_status | CREATE INDEX ... USING btree (status) ``` Both live, both matching the new declarations. The compiled model DDL (`CreateIndex(...).compile(postgresql)`) was diffed against `pg_get_indexdef` on a scratch database and matches character for character modulo Postgres's canonical formatting (`public.` qualification, `USING btree`, parenthesised predicate). That last check is worth more than `alembic check` passing, and is why it was done separately: **alembic does not diff a partial index's `WHERE` clause**. A model declaring `billing_profile_invoice_prefix_key` *without* `deleted_at IS NULL` would also have satisfied the gate — while silently disagreeing with production about whether retiring a billing profile releases its invoice prefix for reuse. The predicate is load-bearing and is now stated in the model with the reasoning attached. ## Deliverable 1 — the CI jobs New `Tests` job, `runs-on: ci`, gated on `detect-changes.outputs.has_code` like its siblings. It runs the **full** suite — 180 tests, including all 22 DB-backed ones — against a `pgvector/pgvector:pg16` service container on `--tmpfs`, mirroring the pattern already proven in `haskos-engineering`. Steps: `uv sync` → `alembic upgrade head` → `alembic check` → `pytest tests/ -q -rs`. Three decisions worth reviewing: - **`alembic check` lives here rather than in its own job** because it needs a migrated database, which this job already has. It is only safe at all because `alembic/env.py` filters autogenerate to `OWNED_TABLES` — unfiltered, it proposes dropping 52 tables belonging to five other applications. - **A skip fails the job.** The suite's only skip mechanism is `skipif(not HASKOS_TEST_DATABASE_URL)` on the DB tier, so a skip in CI means that tier vanished rather than a test being excused. The `Migrations` step cannot catch this — it reads `HASKOS_DATABASE_URL`, a *different* variable, so a typo in the test URL leaves the schema built, migrations green, and 22 tests quietly gone. That is the shape `haskos-engineering` shipped (`432 passed, 189 skipped` in CI against `618 passed` locally), and it survived because the number on the left is the one people read. No `|| true` anywhere; verified by running the step's exact shell with the variable unset — it exits 1. - **`deploy` now `needs: [test, docker-build]`.** With `needs: [docker-build]` alone, a red suite would have deployed to prod beside a green build and the gate would have been decorative. `needs:` treats skipped as not-satisfied, so a docs-only push still skips the deploy exactly as it does today. `HASKOS_DEBUG: "true"` is set on this job and is deliberately *not* set on the smoke test below it. The smoke test exists to boot the way production boots, so it must not skip the secret validation production runs. This job exercises the domain layer against a scratch database and has no production shape to reproduce — `haskos_kernel.config` would otherwise reject the placeholder secrets at import and never reach a test. ## Spec Drift Callouts 1. **Test count is 180, not ~265.** 158 dependency-free + 22 DB-backed, across three files. 2. **`UV_INDEX_HASKYTECH_PASSWORD` alone is not enough.** The brief says to mirror the Docker Build job, which passes only the password — that works there because the *Dockerfile* supplies `UV_INDEX_HASKYTECH_USERNAME=__token__`. A bare `uv sync` on the runner has no Dockerfile behind it, so this job sets both. Copying Docker Build literally would have failed to resolve `haskytech-haskos-kernel`. 3. **No fallback tier was needed.** The brief offered a dependency-free-only fallback if the service container proved fragile. It did not: the full suite including all 22 DB tests passes, and is re-runnable against the same database (the tests delete exactly the rows they wrote). 4. **The skip guard is an addition, not in the brief.** Judged in scope given the explicit "no `|| true`" instruction — a suite that silently skips its DB tier is a suppressed gate wearing a passing summary line. 5. **`deploy` needing `test` is an addition, not in the brief.** Same reasoning: without it the new gate does not gate the deploy. ## Out of scope, flagged not fixed - `Dockerfile` `HEALTHCHECK` uses `curl -f` with `--start-period=30s`. Org convention is `wget -qO /dev/null` and 120s+ for a Python app. Untouched — unrelated to either deliverable, and changing the health probe of a deployed app is not a hygiene edit. - The new `Tests` job will not be a **required** status context until branch protection is updated for this repo; it blocks nothing on its own. - `~/bin/haskos-db` is broken — it discovers databases from the Coolify API and the fleet has moved to Dokku, so it dies in `json.load` on an empty response. The prod verification above went via `ssh contabo-sg` + `docker exec` instead. ## Validation | Check | Result | |---|---| | `ruff check app/ tests/ scripts/` | pass | | `ruff format --check app/ tests/ scripts/` | 46 files already formatted | | `uv run pytest tests/ -q` | **180 passed, 0 skipped** | | `uv run alembic check` | `No new upgrade operations detected.` | | `uv run alembic heads` | `c5e19a7d3f60 (head)` — one head, unchanged | | Migrations added | **none** | | Skip guard fires when DB tier absent | verified, exits 1 | | Model DDL vs `pg_get_indexdef` | equivalent, both indexes | | Prod index existence | confirmed on `contabo-sg` | | `ci.yml` parses | `yaml.safe_load`, jobs and `needs` asserted | Run against local PostgreSQL 17, scratch database `haskos_fin_ci`. **Do not merge without review — merging auto-deploys to production.**

john added 1 commit

2026-08-03 23:30:37 +00:00

ci: pytest + alembic check gates; reconcile index drift

CI / Detect Changes (pull_request) Successful in 6s

Details

CI / Backend (pull_request) Successful in 8s

Details

CI / Tests (pull_request) Successful in 26s

Details

CI / Docker Build (pull_request) Successful in 34s

Details

CI / Deploy (pull_request) Has been skipped

Details

918358f3e7

The repo carried 180 tests over the commitment transition guards and the
recurrence engine, and CI ran ruff plus a Docker smoke test. A regression in
either would have merged green.

Add a Tests job on a postgres service container so the full suite runs,
including the 22 database-backed tests that prove uq_commitment_event_period —
the constraint that makes a recurring commitment bill a period exactly once.
That guarantee is a database object, so the dependency-free tier cannot assert
it. The job fails on skips as well as failures: the only skip mechanism in this
suite is the DB-tier env guard, and in CI its firing means the tier vanished.

alembic check runs in the same job, after upgrade head, and it now passes.
Two indexes existed in production and in no model:

  billing_profile_invoice_prefix_key  partial unique, WHERE deleted_at IS NULL
  ix_invoice_status                   plain btree on invoice(status)

Both were created by this repo's own migrations (f64a7ec6edcf and 001), so the
migrations are the truthful side and the models had drifted away from them.
Aligned the models. No migration added; alembic heads is unchanged at one.
Verified against contabo-sg: both indexes are live there and the compiled model
DDL matches pg_get_indexdef character for character.

deploy now needs the test job. With needs: [docker-build] alone a red suite
would have deployed beside a green build.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KR2UqnKv56zQ1Xffv2QrVt