Merge pull request 'doc: CLAUDE.md and modernization spec' (#4) from docs/claude-md into develop
Reviewed-on: https://git.zavage.net/Zavage-Software/app_skellington/pulls/4
This commit is contained in:
zavage
@@ -0,0 +1,80 @@
|
||||
# CLAUDE.md
|
||||
|
||||
This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository.
|
||||
|
||||
## What this is
|
||||
|
||||
`app_skellington` is a small Python library (published to PyPI) for building CLI applications. It has nothing runnable on its own. It glues together argparse-based multi-level command menus, name-based dependency injection, ConfigObj INI config with spec validation, and colorlog logging. It targets Python >=3.8 on Linux, Windows and Mac. Runtime deps are `appdirs`, `configobj` and `colorlog`.
|
||||
|
||||
## Commands
|
||||
|
||||
```bash
|
||||
pip install -e '.[dev]' pytest # dev extras = black, isort, flake8, pre-commit; pytest is NOT in the extras
|
||||
pytest tests # full suite from repo root (or `pytest .` from inside tests/)
|
||||
pytest tests/cfg/test_cfg.py::TestConfig_e2e::test_uses_spec_as_defaults # single test
|
||||
pytest tests -k "<keyword>"
|
||||
|
||||
black app_skellington
|
||||
isort --profile black app_skellington
|
||||
flake8 app_skellington # .flake8: max-line-length 120, tests/ excluded
|
||||
|
||||
python -m build # sdist + wheel into dist/
|
||||
python -m setuptools_scm # print the version the build will get
|
||||
```
|
||||
|
||||
- **Versioning is tag-driven.** setuptools_scm uses `release-branch-semver` and derives the version from the latest `vX.Y.Z` git tag. It writes `app_skellington/_version.py` at build time. That file is tracked, but don't hand-edit it. `pip install -e .` also rewrites it, so `git restore` it rather than committing the churn. A release means tagging, then `python -m build`, then `twine upload dist/*`. The README covers the full publish steps.
|
||||
- **Formatter config is inconsistent.** `pyproject.toml` has no `[tool.black]`, so black runs at its default width of 88. Only flake8 allows 120. The code on `main` is formatted at 88. The pre-commit black hook pins `language_version: python3.8`. It fails unless a 3.8 interpreter is on PATH (`.python-version` names 3.8.x for pyenv).
|
||||
- **Lint is not currently clean on `main`.** black wants to reformat 2 files, isort flags 3, and flake8 reports W391 and W503. Keep unrelated reformatting out of feature diffs.
|
||||
- Tests cover only `cfg` (ConfigObj fixtures live in `tests/cfg/`) plus a CommandTree constructor smoke test. `tests/test_log.py` is empty.
|
||||
|
||||
## Python support policy
|
||||
|
||||
`requires-python` is `>=3.8`, and the claim should match what CI actually tests. When an end-of-life Python version starts failing, drop it: raise `requires-python` and remove its CI lane. Don't add compat shims, version branches or contorted code just to keep it working. Clean code matters more than supporting EOL runtimes. Users on old Pythons keep the older releases on PyPI.
|
||||
|
||||
## Architecture
|
||||
|
||||
Package layout: `_bootstrap.py`, `_util.py`, `cfg.py`, `log.py`, `cli.py` and `app_container.py`. `__init__.py` re-exports the public names explicitly.
|
||||
|
||||
**`ApplicationContainer` (app_container.py) is the entry point applications subclass.** Its `__init__` does the following, in order:
|
||||
1. builds `cfg.Config(configspec_filepath, configini_filepath)`
|
||||
2. builds `log.LoggingLayer` and configures it from the config's `[logging]` section, or from `DEFAULT_LOG_SETTINGS` if there is none
|
||||
3. creates `ApplicationContext` (`self.ctx`, which holds config, log and parsed argv)
|
||||
4. registers `ctx` as a service and creates `self.cli = CommandTree()`
|
||||
5. calls the optional subclass hooks **`_cli_options()`, `_services()`, `_command_menu()`**, in that order, but only if they are defined
|
||||
|
||||
Applications then call `invoke_from_cli()`, which parses argv and dispatches.
|
||||
|
||||
**Dependency injection works by parameter name.**
|
||||
- Services are registered as *factories*: `app["db"] = lambda: Db(...)`.
|
||||
- `app["db"]` calls the factory each time, so factories decide whether services are singletons.
|
||||
- An unknown name raises `ServiceNotFound`.
|
||||
- `_inject_service_dependencies(cls)` reads `cls.__init__`'s parameter names (skipping `self`) and returns a partial. When that partial is called, it resolves each name from the container and constructs the class.
|
||||
|
||||
**Classes become commands.** `_util.register_class_as_commands(app, submenu, Cls)` registers every public function of `Cls` as a command. Each invocation constructs a *fresh* `Cls` instance with its dependencies injected, then calls the method. The method's signature (minus `self`) and its docstring are what argparse sees. You pass the class, not an instance.
|
||||
|
||||
**`CommandTree` (cli.py) maps function signatures onto argparse.**
|
||||
- Parameters with defaults become optional positionals (`nargs="?"`). Parameters without defaults become required positionals. Options can be added with `add_argument`.
|
||||
- Help text is the first sentence of the docstring (`HelpGenerator`).
|
||||
- It runs in one of two **mutually exclusive modes**. In submenu mode, `init_submenu()` → `SubMenu.create_submenu()` / `register_command()` fills nested subparsers, tracked in `entries`. In single-command mode, `CommandTree.register_command()` binds one function directly to the root parser. The two modes are guarded by asserts in `_lookup_command`.
|
||||
- Dispatch walks the parsed namespace one submenu `var_name` at a time until it hits a `CommandEntry`. It then calls the callback with the args whose names match the signature.
|
||||
- argparse placement matters: `prog --opt sub cmd` applies `--opt` at the root, while `prog sub cmd --opt` applies it to the command.
|
||||
|
||||
**`ServiceNotFound` and `NoCommandSpecified` live in `_util.py`.** That placement avoids a circular import between `cli` and `app_container`. Don't move them back into `app_container`.
|
||||
|
||||
**Config (cfg.py)** wraps `configobj.ConfigObj`.
|
||||
- Setting `configspec_filepath` or `configini_filepath` reloads and revalidates immediately.
|
||||
- Validation runs with `copy=True`, so spec defaults populate the config.
|
||||
- Interpolation is `"template"` (`$var`, not `%(var)s`).
|
||||
- Keys beyond the spec are allowed.
|
||||
- A validation failure logs and returns `False`; it doesn't raise. The strict-validation flag is hard-coded off, and the `capabilities` constructor argument is ignored.
|
||||
- `EnvironmentVariables` is an unimplemented stub.
|
||||
|
||||
**Logging (log.py, _bootstrap.py).**
|
||||
- The framework's own logs go to a private logger named `skell` (`_bootstrap_logger`). It is silenced to CRITICAL with `propagate=False`, unless the env var **`APPSKELLINGTON_DEBUG`** is set. The README's `APPSKELLINGTON_ENABLE_LOGGING` is stale; the code only reads `APPSKELLINGTON_DEBUG`.
|
||||
- That env var is checked twice: once at import in `_bootstrap.py`, and again in `LoggingLayer._add_own_logconfig` when the logging config is (re)applied.
|
||||
- `LoggingLayer.transform_config` adapts a ConfigObj-friendly dict for `logging.config.dictConfig`. It forces `version=1` and turns level strings (`"debug"`, `"all"`, ...) into ints. It renames the logger key `root` to `""`, because ConfigObj can't hold an empty key. It also resolves a relative `handlers.file.filename` into `appdirs.user_log_dir(appname, appauthor)`.
|
||||
- `_bootstrap.py` also checks at import time that the third-party deps are installed, and raises `ImportError` if they're missing.
|
||||
|
||||
## Branches
|
||||
|
||||
`develop` is the integration branch: feature branches come off it, and PRs target it. Changes reach `main` through a `develop` → `main` PR. Release tags (`vX.Y.Z`) go on `main`. In 2026-10, `develop` was recreated from `main`; the 2024 `develop` and its PR #2 were superseded. Modernization roadmap: `docs/superpowers/specs/2026-10-04-modernization-design.md`.
|
||||
+18
-14
@@ -1,20 +1,24 @@
|
||||
# file generated by setuptools-scm
|
||||
# file generated by vcs-versioning
|
||||
# don't change, don't track in version control
|
||||
from __future__ import annotations
|
||||
|
||||
__all__ = ["__version__", "__version_tuple__", "version", "version_tuple"]
|
||||
|
||||
TYPE_CHECKING = False
|
||||
if TYPE_CHECKING:
|
||||
from typing import Tuple, Union
|
||||
|
||||
VERSION_TUPLE = Tuple[Union[int, str], ...]
|
||||
else:
|
||||
VERSION_TUPLE = object
|
||||
__all__ = [
|
||||
"__version__",
|
||||
"__version_tuple__",
|
||||
"version",
|
||||
"version_tuple",
|
||||
"__commit_id__",
|
||||
"commit_id",
|
||||
]
|
||||
|
||||
version: str
|
||||
__version__: str
|
||||
__version_tuple__: VERSION_TUPLE
|
||||
version_tuple: VERSION_TUPLE
|
||||
__version_tuple__: tuple[int | str, ...]
|
||||
version_tuple: tuple[int | str, ...]
|
||||
commit_id: str | None
|
||||
__commit_id__: str | None
|
||||
|
||||
__version__ = version = "0.3.0.dev7+gd155176.d20250223"
|
||||
__version_tuple__ = version_tuple = (0, 3, 0, "dev7", "gd155176.d20250223")
|
||||
__version__ = version = '0.3.0.dev10+gda9568b4f.d20261005'
|
||||
__version_tuple__ = version_tuple = (0, 3, 0, 'dev10', 'gda9568b4f.d20261005')
|
||||
|
||||
__commit_id__ = commit_id = 'gda9568b4f'
|
||||
|
||||
File diff suppressed because it is too large
Load Diff
@@ -0,0 +1,113 @@
|
||||
# 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.
|
||||
Reference in New Issue
Block a user