From 54043a97d2cdd7b339576248b8069973acbdef04 Mon Sep 17 00:00:00 2001 From: patrick Date: Mon, 17 Aug 2026 15:17:20 -0400 Subject: [PATCH] feat: check formatting, linting and tests in CI Every push runs ruff, prettier, pytest and svelte-check; a tagged release runs the blocking half again before it publishes an image. eslint and svelte-check report without failing, since 87 and 30 findings predate the workflow. --- .gitea/workflows/ci.yml | 80 ++++++++++++++++++++++++++++++++++++ .gitea/workflows/release.yml | 35 ++++++++++++++++ AGENTS.md | 5 +++ TODO.md | 14 +++---- docs/ci-release-pipeline.md | 50 +++++++++++++++++----- frontend/AGENTS.md | 9 ++-- 6 files changed, 172 insertions(+), 21 deletions(-) create mode 100644 .gitea/workflows/ci.yml diff --git a/.gitea/workflows/ci.yml b/.gitea/workflows/ci.yml new file mode 100644 index 0000000..fc0e1c9 --- /dev/null +++ b/.gitea/workflows/ci.yml @@ -0,0 +1,80 @@ +name: ci + +# Formatting, linting, types and tests on every push and pull request. +# +# Blocking: the checks that are clean today — ruff format, ruff check, prettier, pytest. +# Non-blocking: eslint and svelte-check, which still report 87 and 30 pre-existing errors. +# Those need real code changes rather than a formatter, so they report without failing the +# build; drop the `continue-on-error` line from a step once its count reaches zero. +# +# The release workflow runs the blocking half again before it publishes anything. + +on: + push: + branches: ['**'] + pull_request: + +jobs: + backend: + runs-on: ubuntu-latest + + defaults: + run: + working-directory: backend + + steps: + - uses: actions/checkout@v4 + + # astral-sh/setup-uv is not mirrored on gitea.com, unlike the actions used elsewhere + # here, so install uv directly rather than depending on DEFAULT_ACTIONS_URL. + - name: Install uv + run: | + curl -LsSf https://astral.sh/uv/install.sh | sh + echo "$HOME/.local/bin" >> "$GITHUB_PATH" + + - name: Install dependencies + run: uv sync --locked + + - name: Format + run: uv run ruff format --check src/ + + - name: Lint + run: uv run ruff check src/ + + # pytest-databases starts a throwaway PostgreSQL container, so this needs a working + # Docker daemon on the runner — the same requirement the release workflow has. + - name: Tests + run: uv run pytest tests/ -q + + frontend: + runs-on: ubuntu-latest + + defaults: + run: + working-directory: frontend + + steps: + - uses: actions/checkout@v4 + + - uses: actions/setup-node@v4 + with: + node-version: 24 + + - name: Enable pnpm + run: corepack enable + + - name: Install dependencies + run: pnpm install --frozen-lockfile + + # Split out of `pnpm lint` (which is prettier && eslint) so formatting can block + # while eslint does not. + - name: Format + run: pnpm exec prettier --check . + + - name: Lint + run: pnpm exec eslint . + continue-on-error: true + + - name: Types + run: pnpm check + continue-on-error: true diff --git a/.gitea/workflows/release.yml b/.gitea/workflows/release.yml index 03f49b0..2e2cd58 100644 --- a/.gitea/workflows/release.yml +++ b/.gitea/workflows/release.yml @@ -23,7 +23,42 @@ env: REGISTRY: git.jaroszew.ski jobs: + # The blocking half of ci.yml, run again on the tagged commit so a release cannot publish + # an image whose tests fail. Deliberately duplicated rather than shared: Gitea's support + # for reusable workflows is thinner than GitHub's, and this is a dozen lines. + # Keep in step with ci.yml. + quality: + runs-on: ubuntu-latest + + steps: + - uses: actions/checkout@v4 + + - name: Install uv + run: | + curl -LsSf https://astral.sh/uv/install.sh | sh + echo "$HOME/.local/bin" >> "$GITHUB_PATH" + + - name: Backend format, lint and tests + working-directory: backend + run: | + uv sync --locked + uv run ruff format --check src/ + uv run ruff check src/ + uv run pytest tests/ -q + + - uses: actions/setup-node@v4 + with: + node-version: 24 + + - name: Frontend format + working-directory: frontend + run: | + corepack enable + pnpm install --frozen-lockfile + pnpm exec prettier --check . + build: + needs: quality runs-on: ubuntu-latest strategy: diff --git a/AGENTS.md b/AGENTS.md index 61a0be2..db9118a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -103,6 +103,11 @@ API docs are served by the running backend at `http://localhost:8000/schema/` (S manually against a live backend). - **Migrations are mandatory.** The app runs with `create_all=False`, so a model change without a matching Alembic revision will not reach the database. +- **CI gates formatting, linting and tests.** `.gitea/workflows/ci.yml` runs on every push and pull + request: `ruff format --check`, `ruff check`, `pytest`, and `prettier --check` all **block**; + `eslint` and `pnpm check` report without failing until their pre-existing counts reach zero. A + `v*` tag additionally builds and publishes both container images — see + `docs/ci-release-pipeline.md`. - **Commit messages** follow `type: summary` — `feat:`, `fix:`, `refactor:`, `chore:`. - The three `README.md` files are user-facing. Agent-facing knowledge belongs in the `AGENTS.md` files. diff --git a/TODO.md b/TODO.md index b7ed6cb..4daebfe 100644 --- a/TODO.md +++ b/TODO.md @@ -114,15 +114,15 @@ CMD ["litestar", "--app-dir", "chitai", "run", "--host", "0.0.0.0", "--port", "8 `litestar run` is the CLI development runner. Production should invoke uvicorn or granian directly, with a worker count. -### Nothing gates formatting, linting or types +### No type checker on the backend -`ruff format --check src/` reports 27 of 61 files unformatted, and `ruff check src/` finds -114 errors — 100 of them unused imports, the rest bare `except`, unused variables and -`== True` comparisons. `ruff check --fix` clears 51 automatically. +Formatting, linting and tests now run in CI (`.gitea/workflows/ci.yml`) and block, and the +tree is clean against them. What is still missing is a type checker: nothing runs one despite +`# type: ignore` comments in the tree. -There is no `[tool.ruff]` section in `pyproject.toml`, so only ruff's default `E4/E7/E9/F` -rules run, and no type checker is configured at all despite `# type: ignore` comments in -the tree. Individually these are trivial; collectively they say nothing runs on commit. +`pyproject.toml` gained a `[tool.ruff.lint.per-file-ignores]` section for `__init__.py` +re-exports, but no rule selection — so only ruff's default `E4/E7/E9/F` rules run. Widening +that set is worthwhile and will surface a fresh batch of findings. ## Frontend diff --git a/docs/ci-release-pipeline.md b/docs/ci-release-pipeline.md index 877cb0f..bb9590c 100644 --- a/docs/ci-release-pipeline.md +++ b/docs/ci-release-pipeline.md @@ -144,20 +144,48 @@ made there: Dockerfiles' `--mount=type=cache` blocks do nothing across ephemeral runners either way, and a cold build of both images is a few minutes. -## Gating: what the release job should and should not run +## Gating: formatting, linting, types and tests -Do **not** put `pytest`, `pnpm check` or `pnpm lint` in front of the push. Per `TODO.md` and -`frontend/AGENTS.md` those are all currently red: `ruff check src/` reports 114 errors, -`pnpm check` has a documented baseline of 30 errors, `pnpm lint` 131. Wiring them into the release -path means the first tag fails for reasons that have nothing to do with the release, and the -predictable response is to disable the gate. Cleaning those up is worthwhile and is its own task — -`TODO.md` already tracks it as "Nothing gates formatting, linting or types". +`ci.yml` runs these on every push and pull request, and `release.yml`'s `quality` job runs the +blocking half again on the tagged commit, so a release cannot publish an image whose tests fail. -Also skip `pytest` here specifically: it needs Docker for `pytest-databases`, which means -docker-in-docker-in-docker on the runner, for a suite that tests code paths the image build does not -affect. +My first draft of this document argued for keeping all of it out of the release path, on the +grounds that everything was red. Measuring rather than trusting `TODO.md` showed that was mostly +wrong: -What *is* worth gating on is that the images actually start, which is the failure mode a release +| Check | Before | After the sweep | Gate | +| --- | --- | --- | --- | +| `pytest tests/` | **411 passed** in 3 min | unchanged | blocking | +| `ruff format --check src/` | 27 of 66 files | clean | blocking | +| `ruff check src/` | 110 errors | clean | blocking | +| `prettier --check .` | 48 files | clean | blocking | +| `eslint .` | 1804 → **87 real** | 87 | non-blocking | +| `pnpm check` | 30 errors | 30 | non-blocking | + +The test suite was green the whole time, so gating on it costs nothing. `ruff check`'s 110 were +104 unused imports, and **all 64 that survived `--fix` were in `__init__.py`** — deliberate +re-exports, so the fix is `per-file-ignores` in `pyproject.toml`, not deleting them. Of eslint's +1804, **1717 were the vendored pdf.js under `static/`**, which the config never ignored the way it +ignores `src/lib/vendor/`; adding it leaves 87 real ones. + +Two things the sweep turned up that are worth remembering: + +- **ruff's suggested fix for `E712` would have broken the query.** `services/filters/book.py` had + `m.BookProgress.completed == True`, and ruff proposes `if m.BookProgress.completed:` — Python + truthiness on a SQLAlchemy Column, which does not generate SQL at all. The correct idiom is + `.is_(True)`, which the adjacent line already used. Never run `ruff check --fix --unsafe-fixes` + over query-building code without reading every hunk. +- **Prettier reformatted the generated `schema.d.ts`**, which was 3109 of the sweep's 3946 changed + lines. `openapi-typescript` writes its own style, so that file would have churned by thousands of + lines on every regeneration — and then failed the very gate being added. It is in + `.prettierignore` now. + +`eslint` and `pnpm check` run with `continue-on-error: true`. That is an honest weak gate: it +reports without failing, so the counts stay visible and cannot grow silently unnoticed, but nobody +is blocked by 117 pre-existing problems they did not create. Drop the line from each step as its +count reaches zero. + +The other thing worth gating on is that the images actually start, which is the failure mode a release introduces and which nothing else catches. That is the `smoke` job in the same workflow: it copies `.env.prod-example`, pins `CHITAI_VERSION` to the tag, `docker compose pull`s the images that were just pushed and brings the stack up with `--wait`, then curls both healthchecks. diff --git a/frontend/AGENTS.md b/frontend/AGENTS.md index 60f01ab..ae38c2c 100644 --- a/frontend/AGENTS.md +++ b/frontend/AGENTS.md @@ -163,9 +163,12 @@ Observed in the current tree — don't mistake these for intentional patterns to current; regenerate it again after any backend API change, with `pnpm exec openapi-typescript http://localhost:8000/schema/openapi.json -o src/lib/schema/openapi/schema.d.ts` against a backend running **your** branch — a stale server silently writes a stale file. -- `pnpm lint` does not pass either — 131 pre-existing ESLint errors, 35 of them - `svelte/no-navigation-without-resolve` on plain `href`s, plus ~59 files Prettier would rewrite - (mostly vendored shadcn components). Check the files you touched, not the whole tree. +- **Prettier is clean and CI blocks on it** — run `pnpm format` before finishing. Two things it + must not touch are in `.prettierignore`: the vendored foliate-js, and + `src/lib/schema/openapi/schema.d.ts`, which `openapi-typescript` regenerates in its own style. +- `pnpm exec eslint .` reports **87 pre-existing errors as of 2026-08-17**, all in `src/`. CI runs + it non-blocking (`continue-on-error`) until that reaches zero. `static/pdfjs/` is ignored + alongside `src/lib/vendor/` — it is vendored too, and linting it produced 1717 further errors. - `src/routes/api/[...path]/+server.ts` — all four handlers are annotated `RequestHandler` while the import of that type is commented out at line 4. It also buffers whole **responses** with `arrayBuffer()` and forwards no `Range` header, so book downloads are not streamed. **Requests**