app_skellington/docs/superpowers/specs/2026-10-04-modernization-design.md

# app_skellington modernization — design

Date: 2026-10-04 · Status: draft for review

## Goal

Keep the library's existing behaviour while modernizing the tooling and backing every compatibility claim with CI. After that, harden the code. The first user-visible result is a **0.2.3 hotfix release**. Every install of the current PyPI release (0.2.2) is broken: `import app_skellington` raises `ImportError ... partially initialized module ... (most likely due to a circular import)`. The fix (`d155176`) is already on `main` but was never released.

## Decisions (agreed in brainstorming)

| Topic | Decision |
|---|---|
| Python floor | Keep `requires-python >=3.8`. When an EOL version starts failing, drop it rather than contort the code (policy recorded in CLAUDE.md). |
| Platforms | Claim only what's tested: **Linux**, CPython 3.8–3.14. Windows and macOS are a stated long-term goal for this repo, but out of scope until a runner exists. |
| Formatter/linter | **ruff** replaces black, isort and flake8. |
| Line length | **88**: the black and ruff default and the de facto modern standard. `main` is already formatted at 88. Measured churn: 4 files, cosmetic only (assert-message wrapping). |
| Reformatting | Goes in its **own style-only PR**. |
| Build system | Keep pyproject + setuptools + setuptools_scm (tag-driven versions). Add **uv** for envs, the lockfile, builds and publishing. Poetry is already gone from `main`. |
| PyPI | Nothing gets published until CI is green across the matrix. Each release goes to TestPyPI first. Publishing stays manual (Mathew's credentials); there's no publish job in CI for now. |
| Branches | Mathew recreates `develop` from `main`. All PRs below target `develop`, `develop` goes to `main` by PR, and release tags go on `main`. Stale PR #2 (develop → main, 2024) gets closed as superseded. |

## Current state (verified 2026-10-04)

- The suite passes on 3.8, 3.9, 3.10, 3.12, 3.13 and 3.14 from `main`, but it covers only `cfg` plus one CommandTree constructor smoke test. Passing it says little about CLI, DI or logging.
- `app_skellington/_version.py` is tracked, yet setuptools_scm rewrites it on every install or build, so every dev run leaves a dirty tree. (The generated file uses `from __future__ import annotations`, and running it on 3.8 was verified to work.)
- Lint isn't clean: black would reformat 2 files, isort flags 3, flake8 reports W391 and W503.
- The README names the wrong debug env var (`APPSKELLINGTON_ENABLE_LOGGING`; the code reads `APPSKELLINGTON_DEBUG`). It also claims Windows and Mac support that nothing tests.

## Delivery: five PRs in order, then the release

### PR 1: style (ruff, format only)

- `pyproject.toml`: `[tool.ruff]` with `line-length = 88` and `target-version = "py38"`, excluding `app_skellington/_version.py`. Lint rules are `select = ["E", "F", "W", "I"]`, matching the old flake8 + isort coverage.
- Any existing violation that `ruff check --fix` can't fix safely gets a per-rule `ignore` with a comment. Each one is removed during hardening (PR 5). This PR changes no logic.
- Apply `ruff format` and the safe `ruff check --fix` (import order).
- Delete `.flake8`. In `.pre-commit-config.yaml`, replace the black, isort and flake8 hooks with `ruff-pre-commit` (`ruff-check --fix`, `ruff-format`). That also drops the `python3.8` pin, which breaks on machines without 3.8.
- Done when `ruff check` and `ruff format --check` are clean, tests pass, and the diff is whitespace and import order only.

### PR 2: tooling (uv), packaging hygiene, docs

- `[dependency-groups] dev = ["ruff", "pytest", "pre-commit", "pyyaml"]` (PEP 735) replaces `[project.optional-dependencies] dev`, so dev tools are no longer extras on the published package. Commit `uv.lock`. uv resolves one lock across all Pythons in range, so 3.8 lanes get compatible older pytest and other tools with no hand pinning.
- Move `tests/pytest.ini` into `[tool.pytest.ini_options]` (with `testpaths = ["tests"]`), so plain `uv run pytest` from the repo root picks up the config.
- `.python-version`: set the dev default to one current version (3.14). CI covers the rest.
- **Untrack `_version.py`** and add it to `.gitignore`. setuptools_scm still generates it at build time, so the sdist and wheel ship it. Nothing in the package imports it.
- Commands become `uv sync`, `uv run pytest`, `uv run ruff ...`, `uv build` and `uv publish`.
- README:
  - fix the debug env var
  - replace the pyenv and pip dev setup with uv
  - drop the untested Windows/Mac claim; PR 3 adds the tested-platform line once CI backs it
  - update the test and publish instructions
  - leave the claim "config.ini is created if missing" until PR 4 confirms it; correct it there if the test disagrees
- Update CLAUDE.md to match: commands, the branch section after `develop` is recreated, and the lint and `_version.py` notes.

### PR 3: Gitea CI (checks only)

`.gitea/workflows/ci.yaml`, following `~/.claude/docs/gitea-ci-pattern.md`:

- **Triggers:** `pull_request` (types `opened, synchronize, reopened, edited`), `push: [develop, main]`, `workflow_dispatch`. Concurrency cancels superseded PR runs only.
- **WIP gate** on every job that runs on `pull_request`: skip when the title starts with `WIP:` or `[WIP]`, and nothing else.
- **`lint`:** one Python. Runs `uv run ruff check` and `uv run ruff format --check`.
- **`test`:** matrix over `python: ["3.8", "3.9", "3.10", "3.11", "3.12", "3.13", "3.14"]`, `fail-fast: false`. Container `python:<v>`. The first step installs `nodejs` (Gitea actions are node programs), then uv. Checkout uses `fetch-depth: 0`, because setuptools_scm needs tags. Then each lane:
  1. `uv sync --locked`, then `uv run pytest`.
  2. **Packaging smoke test:** `uv build`, install the built wheel into a fresh venv, then `cd /` (away from the source tree) and run `python -c "import app_skellington; ..."`. This is the check that would have caught 0.2.2.
- **`ci-ok`** aggregate job, `needs: [lint, test]`, with `if: always()` plus the WIP gate. It fails unless every needed job's result is `success`.
  - Why it runs `always()`: Gitea reports a *skipped* job as a `success` status. A plain `needs:` aggregate would go green when a needed job failed. The `always()` + explicit check closes that hole.
  - Branch protection then requires only `CI / ci-ok (pull_request)`, so one required check covers every matrix lane.
- **`tests/test_ci_workflow.py`** pins the invariants: triggers, WIP gate on each PR job, matrix versions equal to the `requires-python` floor through the newest, and the `ci-ok` logic. CI drift then shows up as a red test.
- README: add "Tested on Linux, CPython 3.8–3.14 (CI). Windows/macOS: goal, untested."
- **Rollout per the playbook:** run every gate locally first, then canary one gate at a time (a lint failure, then a test failure), watch it go red, revert, watch it go green. For each canary, confirm that **`CI / ci-ok (pull_request)` itself** reports failure, not just the broken lane. This proves `if: always()` and `needs.<job>.result` evaluate as expected on this act_runner, since the pattern doc never canaried them. Mathew then adds the branch-protection status check.

### Release 0.2.3 (hotfix, after PR 3 is green on `main`)

1. Promote `develop` → `main` by PR, with CI green.
2. Tag `v0.2.3` on `main`, then `uv build`.
3. `uv publish` to **TestPyPI**. Install it from TestPyPI on every matrix Python (a script under `scripts/` running `uv run --isolated --python X --index testpypi`) and import it.
4. `uv publish` to PyPI. Mathew does this step; whether to yank 0.2.2 afterwards is his call.
5. CHANGELOG entry.

### PR 4: characterization tests

These pin *current* behaviour before any hardening changes it. Known bugs get tests marked `xfail(strict=True)`, each with the bug named, so a fix flips the test and forces it to be updated.

- **CLI:** single-command and submenu dispatch, nested submenus, defaults versus required positionals, help text from the first docstring sentence, `NoCommandSpecified`, option placement (root versus command).
- **Container and DI:** factory registration, `ServiceNotFound`, constructor-parameter-name injection, `register_class_as_commands` (fresh instance per call, public methods only), the hook order `_cli_options` → `_services` → `_command_menu`.
- **Logging:** `transform_config` (`root` → `""`, level strings to ints, `version=1`, relative file path → user log dir, using a tmp dir), `APPSKELLINGTON_DEBUG` on and off.
- **Config:** whether config.ini is created when missing (then correct the README to match), `$var` interpolation, spec defaults, validation failure returning `False`.
- **Public API:** everything `__init__.py` exports can be imported.

### PR 5+: hardening, one fix per PR or small groups

Each fix flips its `xfail` from PR 4. Known so far:
- `Config.__delitem__` recurses forever (it calls `del self[key]`).
- `_util.get_asset` has a broken `isinstance(module, module)` check.
- `cli._lookup_command` can stop at `input("<broken>")`.
- `logger.warn` is deprecated; use `warning`.
- `_load_config_files` swallows broad exceptions.
- `Config.__contains__` returns `None` on error.
- Remove the per-rule ruff ignores added in PR 1.
- Issue #3: appdirs → platformdirs, with a path-parity check.

The public API (the names exported from `__init__.py`) stays stable.

## Out of scope (recorded, not planned)

- Windows and macOS CI runners. This is the long-term multi-platform goal and needs infrastructure.
- A PyPI publish job in CI (would need the deferred credential model in the CI pattern doc).
- A PyPy lane and type checking. Both can be revisited once the matrix is stable.

## Risks

- **Old Python images:** `python:3.8` images are frozen, since 3.8 is EOL. If apt or nodejs installs inside them break, the policy applies: drop the lane and raise the floor rather than build workarounds.
- **Lockfile resolution on 3.8:** some dev tools may have no 3.8-compatible release. uv's fork resolution should handle this; if it can't, that's the trigger to raise the floor.
- **setuptools_scm in CI** needs full history and tags (`fetch-depth: 0`, `safe.directory`). Without them the version falls back to `0.0.0`, and the smoke test should assert that it doesn't.