diff --git a/JOURNAL.md b/JOURNAL.md index 766307f..b9f33c3 100644 --- a/JOURNAL.md +++ b/JOURNAL.md @@ -22,6 +22,71 @@ Entry template: --- +## 2026-08-31 — Installable agent skill (`neworder-skill`) + +**Why** — Coding agents write *neworder* models much more reliably when given a compact, +purpose-built reference than when left to infer the framework's shape from source, stubs or a +partial reading of the docs site. The recurring failures are all framework-specific and cheap to +prevent in writing: forgetting `super().__init__(timeline, seeder)`, mutating the result of +`no.df.transition` in place instead of assigning it back, comparing against `NEVER` with `==`, +looping over agents in python instead of vectorising, and letting `check()` return different +values per MPI process. Bundling the reference in the package makes it available in any downstream +project that installs *neworder*, not just in this repo. Modelled on the equivalent change in +[virgesmith/xenoform-rs#24](https://github.com/virgesmith/xenoform-rs/pull/24). + +**What** — Added [neworder/skill/SKILL.md](neworder/skill/SKILL.md), an agent skill covering the +model lifecycle, the four timeline types, the `MonteCarlo` and `SplitMix64` engines and their +seeding strategies, the `neworder.df` operations, spatial domains, MPI patterns and the framework's +recurring pitfalls. Added [neworder/skill_cli.py](neworder/skill_cli.py), a `neworder-skill` console +script (registered under `[project.scripts]`) with `--install [PATH]` / `--remove [PATH]`, default +`PATH=.agents`; `package_data` in [setup.py](setup.py) ships the skill in the wheel and sdist. Tests +in [test/test_skill_cli.py](test/test_skill_cli.py), a new +[docs/agent-skill.md](docs/agent-skill.md) page with a nav entry in [zensical.toml](zensical.toml), +and a pointer to it from [README.md](README.md). + +**Design decisions** +- **The skill points at the documentation site rather than restating it.** Its header table links + to the overview, tips, examples and developer pages at `neworder.readthedocs.io/en/stable/`, and + the body is deliberately a summary an agent can hold in context, not a second copy of the docs + that would drift out of sync with them. That is also why the user-facing documentation for the + feature is a docs-site page, with only a two-line pointer in `README.md` — the README is + inlined into `docs/index.md` via `include_snippet`, so anything longer would duplicate the new + page on the site's front page. +- **Install by symlink where possible, copy where not.** A symlink to `neworder/skill/` inside the + installed package always matches the version in use, with nothing to keep up to date — the + approach `xenoform-rs` took, and Streamlit's `streamlit skills` before it. That repo could stop + there; this one cannot, because CI (and the classifiers) cover Windows, where `symlink_to` needs + developer mode or elevation. So `_link_or_copy` catches `OSError` and falls back to + `shutil.copytree`, and `--install` over an existing copy refreshes it rather than reporting it + up to date, since a copy — unlike a symlink — goes stale on upgrade. The link target is relative where a + relative path exists, so a symlinked skill survives the project being moved; on Windows there is + no relative path between different drives (`os.path.relpath` raises `ValueError`, caught by CI + with the package on `D:` and the project's temp dir on `C:`), so the link target falls back to + an absolute path there. +- **Ownership is checked before anything is overwritten or deleted.** A symlink is ours if it + resolves to the bundled directory; a directory is ours only if every entry is a file whose name + we ship. Anything else — a user's own file, directory or foreign symlink at the target — is left + untouched and the command exits 1. The subset rule means a copy with a user-added file in it is + no longer considered ours, which is the safe direction to err in. +- **`.agents/skills/` as the default target, overridable by `PATH`.** Matches the sibling + repo and the emerging cross-harness convention, and `neworder-skill --install .claude` covers a + specific harness without needing per-harness detection logic in the installer. +- **A console-script entry point rather than a loose script.** `Path(__file__).parent / "skill"` + resolves from wherever `neworder` is importable, so the script naturally targets the environment + it is invoked from with no `.venv` detection. Verified against a built wheel and sdist that + `package_data` ships `neworder/skill/SKILL.md` and that the entry point is registered — this + matters because `[tool.cibuildwheel]` runs the test suite against the installed wheel, so + `test_skill_cli.py` would fail there if the skill were not packaged. + +**Follow-ups** — The skill's content is maintained by hand and can drift from the docs site; the +type-level details it quotes (method signatures, timeline properties) are the most likely to go +stale, and nothing checks them. If it proves worth it, the tables could be generated from the +stubs, or a test could assert that every method named in the skill exists on the corresponding +class. `--install` targets one directory at a time; multi-harness install (writing both `.agents` +and `.claude`) is deferred until someone asks for it. + +--- + ## 2026-08-01 — SplitMix64.raw (#120) **Why** — `SplitMix64` exposed only `uarray`, so the underlying 64-bit hashes were unreachable, and two things needed them. Seeding an external generator previously meant `MonteCarlo.raw()` or the `as_np` bitgen adapter, both of which tie the external generator to the sequential mt19937 stream — what it gets depends on how many draws were taken before it, so there was no order-independent way to initialise one. And variates `uarray` cannot express (a uniform integer over an arbitrary range, say) need the hash itself, not its image in `U[0,1)`. diff --git a/README.md b/README.md index 19c192e..cfd0206 100644 --- a/README.md +++ b/README.md @@ -66,3 +66,9 @@ pip install neworder[parallel-openmpi,geospatial] ## Documentation To get started first see the detailed documentation [here](https://neworder.readthedocs.io). Then, check out "Hello World" and the other examples. + +### Using an AI coding agent? + +*neworder* ships an installable agent skill - run `neworder-skill --install` and your agent gets a +built-in reference for writing *neworder* models correctly. See +[Agent Skill](https://neworder.readthedocs.io/en/stable/agent-skill/). diff --git a/docs/agent-skill.md b/docs/agent-skill.md new file mode 100644 index 0000000..deca772 --- /dev/null +++ b/docs/agent-skill.md @@ -0,0 +1,63 @@ +# Agent Skill + +*neworder* ships an [agent skill](https://code.claude.com/docs/en/skills) — a compact, purpose-built +reference for AI coding agents (Claude Code and others that read `SKILL.md` files) covering the +model lifecycle, timelines, the `MonteCarlo` and `SplitMix64` engines, the DataFrame operations, +spatial domains, parallel execution and the framework's common pitfalls. + +The skill is a *pointer into these docs*, not a replacement for them: it gives an agent enough to +write a correctly-shaped model without loading the whole site into its context, and links back to +the relevant page here whenever more detail is needed. + +## Installing + +The skill is bundled inside the installed package, and a console script installs it into a project: + +```sh +neworder-skill --install [PATH] # default PATH: .agents +neworder-skill --remove [PATH] # default PATH: .agents +``` + +This creates (or removes) `PATH/skills/neworder`. Run it from the environment *neworder* is +installed in — it's a normal console-script entry point, so it's only on `PATH` while that +virtualenv is active (or via `uv run neworder-skill --install`). + +!!! note "Choosing a target directory" + The default `.agents/skills/` is read by agents supporting the cross-tool convention. Pass an + explicit `PATH` for a specific harness, e.g. `neworder-skill --install .claude` installs to + `.claude/skills/neworder`. + +Where the platform allows it, the skill is installed as a **symlink** to the copy inside the +installed package, so it always matches the version of *neworder* actually in use and there is +nothing to keep up to date. On platforms where symlinks aren't permitted (Windows without +developer mode) the files are copied instead, and re-running `--install` refreshes that copy — +do this after upgrading *neworder*. + +!!! warning "Installing into a version-controlled project" + A symlinked skill points outside the repository, so it will not work for anyone else who + checks it out. Either add `.agents/` to `.gitignore` and have each developer install it, or + commit a copy. + +Both commands refuse to touch anything they don't recognise as their own: an existing file, +directory or symlink at the target that isn't an installed copy of this skill is left alone and +the command exits non-zero. + +## What the agent gets + +The skill's frontmatter tells an agent when to load it — mentions of *neworder*, `no.Model`, +`neworder.run`, the timeline classes, `mc.hazard`/`stopping`/`arrivals`, `no.df.transition`, +`SplitMix64`, `StateGrid` or `neworder.mpi`. Its body covers: + +- the `modify` → `step`/`check` → `finalise` lifecycle, and a minimal working model +- the four timeline types, custom timelines, and open-ended timelines with `halt()` +- the `MonteCarlo` sampling methods, the seeding strategies, and the numpy adapter `as_np` +- `SplitMix64`'s keyed, order-independent draws and when they're preferable +- `no.df.transition`/`transition_conditional`/`unique_index`, including the categorical-dtype and + assign-the-result-back requirements +- spatial domains and edge behaviours +- MPI patterns, and the all-or-nothing failure rule that avoids deadlocks +- the framework's recurring pitfalls: an uninitialised base class, strict C++ typing, `NEVER` + being NaN, and explicit per-agent Python loops + +The source is [`neworder/skill/SKILL.md`](https://github.com/virgesmith/neworder/blob/main/neworder/skill/SKILL.md) +in the repository — fixes and additions are welcome, see [Contributing](./contributing.md). diff --git a/neworder/skill/SKILL.md b/neworder/skill/SKILL.md new file mode 100644 index 0000000..d2cbb35 --- /dev/null +++ b/neworder/skill/SKILL.md @@ -0,0 +1,216 @@ +--- +name: neworder +description: > + Use when writing, editing, running or debugging a microsimulation model built on the neworder + framework — subclassing neworder.Model, defining a timeline, sampling from the MonteCarlo or + SplitMix64 engines, transitioning pandas categorical data with neworder.df, spatial domains, and + MPI/multithreaded parallel runs. Triggers: "neworder", "no.Model", "neworder.run", + "microsimulation", NoTimeline/LinearTimeline/NumericTimeline/CalendarTimeline, mc.hazard, + mc.stopping, mc.arrivals, no.df.transition, SplitMix64, StateGrid, neworder.mpi. +--- + +# Building models with neworder + +`neworder` is a dynamic microsimulation framework: a C++ core (via pybind11) exposed as a Python +module. You supply a `Model` subclass and a timeline; the runtime iterates the timeline, calling +your methods at each step. It is data-agnostic — populations are normally `pandas` DataFrames and +the library functions operate directly on them (and on `numpy` arrays) without copying. + +**The canonical documentation is the project site, .** This skill +is a working reference for the shape of a correct model; when you need detail beyond it, read the +relevant page rather than guessing: + +| Page | What's there | +|------|--------------| +| [Overview](https://neworder.readthedocs.io/en/stable/overview/) | the framework's model, timelines, spatial domains, data/performance notes | +| [Tips and Tricks](https://neworder.readthedocs.io/en/stable/tips/) | seeding strategies, reproducibility, `SplitMix64`, halting, deadlocks, time comparison | +| [Examples](https://neworder.readthedocs.io/en/stable/examples/) | 16 runnable models, each with a walkthrough — the best source of idiom | +| [API Reference](https://neworder.readthedocs.io/en/stable/api/) | note: the API is documented by the package's own type annotations/stubs, not on the site | +| [Developer](https://neworder.readthedocs.io/en/stable/developer/) | building from source, running tests, generating stubs | + +Use `/en/stable/` for the released version (what `pip install neworder` gives you) and +`/en/latest/` for the current `main`. + +## Model lifecycle + +`neworder.run(model)` drives this sequence, and nothing else should advance the timeline: + +1. `modify()` — once, before the run. Optional; typically used to perturb inputs per MPI process. +2. `step()` — once per timestep. **Required.** +3. `check()` — after each `step()`, if implemented. Returning `False` aborts the run. +4. `finalise()` — once, on reaching the end of the timeline. Optional. + +`run` returns `False` if the model failed — check it. Disable `check()` calls globally with +`neworder.checked(False)`; turn on runtime logging with `neworder.verbose()`. Log from a model +with `neworder.log(...)`, which prefixes process/timestep context. + +## Minimal model + +```py +import neworder as no +import pandas as pd + + +class MyModel(no.Model): + def __init__(self, n: int, mortality_rate: float) -> None: + # ESSENTIAL: initialise the base class with a timeline (and optionally a seeder). + # Omitting this is the single most common error — it fails at runtime. + super().__init__(no.LinearTimeline(0.0, 100.0, 100), no.MonteCarlo.deterministic_identical_stream) + self.population = pd.DataFrame(index=no.df.unique_index(n), data={"age": 0.0, "alive": True}) + self.mortality_rate = mortality_rate + + def step(self) -> None: + died = self.mc.hazard(self.mortality_rate, len(self.population)).astype(bool) + self.population.loc[died, "alive"] = False + self.population.age += self.timeline.dt + + def check(self) -> bool: + return bool((self.population.age >= 0.0).all()) + + def finalise(self) -> None: + no.log(f"survivors: {self.population.alive.sum()}") + + +if __name__ == "__main__": + model = MyModel(10000, 0.01) + if not no.run(model): + no.log("model failed") +``` + +`self.mc` (the `MonteCarlo` engine), `self.timeline` and `self.run_state` are provided by the base +class. Vectorise: prefer whole-array `mc`/`numpy`/`pandas` operations over per-agent Python loops. + +## Timelines + +| Class | Use for | +|-------|---------| +| `NoTimeline()` | continuous-time / case-based models evaluated in one instantaneous step | +| `LinearTimeline(start, end, nsteps)` | equally-spaced non-calendar steps; `LinearTimeline(start, step)` is open-ended | +| `NumericTimeline(times)` | explicit, unequally-spaced non-calendar times | +| `CalendarTimeline(start, relativedelta(...), end=...)` | calendar dates (day/month/year steps, ACT/365 year fractions) | + +Properties: `index`, `time`, `dt`, `start`, `end`, `nsteps`, `at_end`. Custom timelines subclass +`no.Timeline` and override `start`, `end`, `time`, `dt`, `_next` (and optionally `__repr__`); +`index` is provided and must not be overridden. `CalendarTimeline` (pure Python, in +`neworder/timeline.py`) is the reference implementation to copy. + +Open-ended timelines stop via `model.halt()` from within `step()`. Note that `halt()` does *not* +return immediately — the rest of `step()` and `check()` still run — and `finalise()` is **not** +called for a halted model; call it explicitly if needed. A halted model can be resumed by passing +it to `no.run` again. + +## Randomness + +### `MonteCarlo` — the model's sequential stream (`self.mc`) + +| Method | Returns | +|--------|---------| +| `ustream(n)` | `n` U[0,1) variates | +| `hazard(p, n)` / `hazard(p_array)` | Bernoulli outcomes (0.0/1.0) for a constant or per-agent probability | +| `stopping(lambda_, n)` / `stopping(lambda_array)` | times to a stopping event, constant hazard rate | +| `arrivals(lambda_, dt, n, mingap)` | arrival times from a non-homogeneous Poisson process | +| `first_arrival(...)` / `next_arrival(...)` | first/subsequent arrivals in a non-homogeneous Poisson process | +| `counts(lambda_, dt)` | event counts | +| `sample(n, cat_weights)` | categorical sampling | +| `raw()` / `state()` / `seed()` | a random 64-bit integer (for seeding other generators) / internal state / the seed | +| `reset()` | re-invoke the seeder | + +Non-arrivals are returned as `neworder.time.NEVER` (NaN) — test with `no.time.isnever(x)`, never +with `==`. `no.time.DISTANT_PAST`/`FAR_FUTURE` are `-inf`/`+inf`. + +Seeding strategies (passed as the second argument to `super().__init__`, a callable returning an +`int` that fits `int32`): `MonteCarlo.deterministic_identical_stream`, +`deterministic_independent_stream` (keyed on MPI rank), `nondeterministic_stream`, or your own. +Identical streams + perturbed inputs → sensitivity analysis; independent streams + identical +inputs → convergence analysis. + +For all of `numpy`'s distributions on neworder's stream, use the adapter `no.as_np(self.mc)`. + +### `SplitMix64` — stateless, keyed draws + +`MonteCarlo` is sequential: a draw depends on how many draws preceded it, so adding, removing or +reordering agents changes every subsequent variate. `SplitMix64` hashes integer keys instead, so +the draw for agent *i* is the same whether or not any other agent was drawn: + +```py +self.rng = no.SplitMix64(no.MonteCarlo.deterministic_identical_stream) +draws = self.rng.uarray(person_ids, no.SplitMix64.hash64("mortality"), self.timeline.index) +``` + +Keys are scalars (context, adding no dimension) or **1-D** integer arrays (each adding one output +dimension); multi-dimensional key arrays raise `TypeError`. `raw(...)` returns the underlying +`int64` hashes instead of U[0,1) — used for seeding (e.g. per-timestep re-seeding of `mc`, or +`np.random.PCG64(rng.raw(np.arange(4), MODEL_ID).view(np.uint64))`; view as `uint64` because +numpy rejects negative entropy). `use_counter=True` makes repeated identical calls differ, but the +counter is not thread-safe — give each thread its own instance, or key on a thread-specific +scalar. Choose `MonteCarlo` for non-uniform sampling (it has no `SplitMix64` equivalent) and +`SplitMix64` for uniform draws that must survive sub-sampling or reordering. + +## DataFrame operations (`neworder.df`) + +- `no.df.unique_index(n)` — `n` index values unique across MPI processes. +- `no.df.transition(mc, matrix, series)` — Markov transition of a **categorical** series. Returns + a new `pd.Categorical`; it does **not** modify in place, so assign the result back: + `df[col] = no.df.transition(model.mc, m, df[col])`. `matrix` rows must follow + `series.cat.categories` order; convert with `.astype("category")` first if needed. +- `no.df.transition_conditional(mc, matrices, group, series)` — as above with a different matrix + per row, selected by a second categorical column (e.g. by age band or sex). Rows whose group is + missing are untouched; every other group category needs an entry in `matrices`. + +`no.df.transition` is orders of magnitude faster than an equivalent Python loop — use it rather +than iterating rows. + +## Spatial domains + +Optional. `Space` (continuous, arbitrary dimension; `move`, `dists`, `dists2`, `in_range`), +`StateGrid` (discrete grid; `count_neighbours`, `shift`, indexing), and `GeospatialGraph` (a +`networkx`/`osmnx` wrapper for shortest paths, isochrones, subgraphs — needs the `geospatial` +extra). Edge behaviour is `no.Edge.UNBOUNDED`, `WRAP`, `CONSTRAIN` or `BOUNCE` (`Space` supports +all; `StateGrid` supports `WRAP` and `CONSTRAIN` only). See the boids, Conway, Schelling and +wolf-sheep examples. + +## Parallel execution + +`no.mpi.RANK`, `no.mpi.SIZE`, `no.mpi.COMM` (rank 0, size 1, `COMM` unused in serial). Requires +one of the `parallel-native`/`parallel-openmpi`/`parallel-mpich` extras and `mpiexec -n `. + +- Failure must be **all-or-nothing**: a blocking communication whose counterpart has already + exited deadlocks the whole run. Have one process compute a shared `check()` result and broadcast + it, or reduce with a logical "and" — never let `check()` return different values per process. +- Use `modify()` for per-rank input perturbation. +- Identically-seeded streams only stay synchronised if each process takes the same number of draws. +- Multithreaded (rather than multiprocess) models: `no.freethreaded()` reports whether the + interpreter is free-threaded, and `no.thread_id()` identifies the calling thread. + +## Gotchas + +- **The base class must be initialised** — `super().__init__(timeline, seeder)`. Skipping it is a + runtime error, not a quiet one. +- **The C++ core is statically typed.** Passing `3.0` where an `int` is expected raises `TypeError` + immediately, and `dtype` matters as much as the outer type. The package ships annotations and + `py.typed`, so a type checker catches most of this — use one. +- **`transition`/`transition_conditional` return new data**, they do not mutate the series. +- **Never advance the timeline yourself**; the runtime does it between steps. +- **`NEVER` is NaN** — `NEVER == NEVER` is `False`; use `no.time.isnever`. +- **`mc.reset()` re-invokes the seeder**, so a non-deterministic seeder gives a *different* stream + after reset, not the original one. +- **Explicit per-agent Python loops are the usual performance bug.** Reach for the vectorised + `mc.*`, `no.df.*` and numpy equivalents first. + +## Running models and examples + +Examples are self-contained directories under `examples/`, entry point `model.py` or `run.py`: + +```sh +python examples/mortality/model.py +mpiexec -n 2 python examples/parallel/model.py # parallel examples +``` + +## Working on neworder itself + +If you are changing this framework rather than using it, the repository's `AGENTS.md` governs the +workflow: C++ core in `src/`, Python package in `neworder/`, type stubs regenerated with +`pybind11-stubgen` whenever the pybind11 bindings change, the full gate suite +(`uv run ruff check`, `uv run ruff format --check`, `uv run ty check neworder examples test`, +`uv run pytest`) green before anything is called done, the affected `examples/` run by hand, docs +under `docs/` updated, and a `JOURNAL.md` entry for every substantive change. diff --git a/neworder/skill_cli.py b/neworder/skill_cli.py new file mode 100644 index 0000000..ded05d7 --- /dev/null +++ b/neworder/skill_cli.py @@ -0,0 +1,120 @@ +import argparse +import os +import shutil +from pathlib import Path + +_SKILL_NAME = "neworder" +_DEFAULT_PATH = ".agents" + + +def _source_dir() -> Path: + """The bundled skill directory, shipped inside this package.""" + return Path(__file__).parent / "skill" + + +def _target_path(root: str) -> Path: + return Path(root) / "skills" / _SKILL_NAME + + +def _is_ours(target: Path, source: Path) -> bool: + """Whether target is a symlink to, or a copy of, the bundled skill - and so safe to replace or remove.""" + try: + if target.is_symlink(): + return target.resolve() == source.resolve() + if target.is_dir(): + shipped = {p.name for p in source.iterdir()} + entries = list(target.iterdir()) + return bool(entries) and all(p.is_file() and p.name in shipped for p in entries) + except OSError: + return False + return False + + +def _link_target(source: Path, target: Path) -> str: + """A relative link target, or an absolute one if there is no relative path (different windows drives).""" + try: + return os.path.relpath(source.resolve(), target.parent.resolve()) + except ValueError: + return str(source.resolve()) + + +def _link_or_copy(source: Path, target: Path) -> str: + # symlinks need developer mode or elevation on Windows, so fall back to copying there + try: + target.symlink_to(_link_target(source, target), target_is_directory=True) + except OSError: + shutil.copytree(source, target) + return "copied" + return "linked" + + +def _install(root: str) -> int: + source = _source_dir() + target = _target_path(root) + target.parent.mkdir(parents=True, exist_ok=True) + + if target.is_symlink() or target.exists(): + if not _is_ours(target, source): + print(f"refusing to overwrite existing file or directory not managed by {_SKILL_NAME}-skill: {target}") + return 1 + if target.is_symlink(): + print(f"already installed and up to date: {target}") + return 0 + # a copy can go stale when the installed package is upgraded, so always refresh it + shutil.rmtree(target) + print(f"refreshed ({_link_or_copy(source, target)}): {target}") + return 0 + + print(f"installed ({_link_or_copy(source, target)}): {target} -> {source}") + return 0 + + +def _remove(root: str) -> int: + source = _source_dir() + target = _target_path(root) + + if not target.exists() and not target.is_symlink(): + print(f"not installed: {target}") + return 0 + if not _is_ours(target, source): + print(f"refusing to remove {target}: not managed by {_SKILL_NAME}-skill") + return 1 + + if target.is_symlink(): + target.unlink() + else: + shutil.rmtree(target) + print(f"removed: {target}") + return 0 + + +def main(argv: list[str] | None = None) -> int: + """Entry point for the `neworder-skill` console script.""" + parser = argparse.ArgumentParser( + prog=f"{_SKILL_NAME}-skill", + description=f"Install or remove the '{_SKILL_NAME}' agent skill in a project.", + ) + group = parser.add_mutually_exclusive_group(required=True) + group.add_argument( + "--install", + nargs="?", + const=_DEFAULT_PATH, + metavar="PATH", + help=f"Install the skill into PATH/skills/{_SKILL_NAME} (default PATH: {_DEFAULT_PATH}).", + ) + group.add_argument( + "--remove", + nargs="?", + const=_DEFAULT_PATH, + metavar="PATH", + help=f"Remove the skill from PATH/skills/{_SKILL_NAME} (default PATH: {_DEFAULT_PATH}).", + ) + args = parser.parse_args(argv) + + if args.install is not None: + return _install(args.install) + return _remove(args.remove) + + +if __name__ == "__main__": + raise SystemExit(main()) diff --git a/pyproject.toml b/pyproject.toml index 5da6166..f484ccc 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -66,6 +66,9 @@ parallel-mpich = [ "mpi4py>=4.1.2" ] +[project.scripts] +neworder-skill = "neworder.skill_cli:main" + [project.urls] "Homepage" = "https://neworder.readthedocs.io/" "Bug Tracker" = "https://github.com/virgesmith/neworder/issues" diff --git a/setup.py b/setup.py index 0c307bb..2e6b785 100755 --- a/setup.py +++ b/setup.py @@ -36,7 +36,7 @@ def list_files(dirs, exts, exclude=None): setup( name="neworder", packages=["neworder"], - package_data={"neworder": ["py.typed", "*.pyi"]}, + package_data={"neworder": ["py.typed", "*.pyi", "skill/*.md"]}, ext_modules=ext_modules, zip_safe=False, ) diff --git a/test/test_skill_cli.py b/test/test_skill_cli.py new file mode 100644 index 0000000..0c44d76 --- /dev/null +++ b/test/test_skill_cli.py @@ -0,0 +1,151 @@ +from pathlib import Path + +import pytest + +from neworder import skill_cli + + +def _symlinks_available(tmp_path: Path) -> bool: + probe = tmp_path / "probe" + try: + probe.symlink_to(tmp_path, target_is_directory=True) + except OSError: + return False + probe.unlink() + return True + + +def _no_symlinks(monkeypatch: pytest.MonkeyPatch) -> None: + def raise_oserror(*_args: object, **_kwargs: object) -> None: + raise OSError("symlinks not permitted") + + monkeypatch.setattr(Path, "symlink_to", raise_oserror) + + +def test_install_creates_skill(tmp_path: Path) -> None: + assert skill_cli.main(["--install", str(tmp_path)]) == 0 + + target = tmp_path / "skills" / "neworder" + assert (target / "SKILL.md").is_file() + if _symlinks_available(tmp_path): + assert target.is_symlink() + assert target.resolve() == skill_cli._source_dir().resolve() + + +def test_install_default_path(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: + monkeypatch.chdir(tmp_path) + assert skill_cli.main(["--install"]) == 0 + + assert (tmp_path / ".agents" / "skills" / "neworder" / "SKILL.md").is_file() + + +def test_install_idempotent(tmp_path: Path) -> None: + assert skill_cli.main(["--install", str(tmp_path)]) == 0 + assert skill_cli.main(["--install", str(tmp_path)]) == 0 + + assert (tmp_path / "skills" / "neworder" / "SKILL.md").is_file() + + +def test_install_refuses_existing_directory(tmp_path: Path) -> None: + target = tmp_path / "skills" / "neworder" + target.mkdir(parents=True) + (target / "keepme.txt").write_text("do not delete") + + assert skill_cli.main(["--install", str(tmp_path)]) == 1 + assert not target.is_symlink() + assert (target / "keepme.txt").read_text() == "do not delete" + + +def test_install_refuses_foreign_symlink(tmp_path: Path) -> None: + if not _symlinks_available(tmp_path): + pytest.skip("symlinks not available on this platform") + foreign = tmp_path / "elsewhere" + foreign.mkdir() + target = tmp_path / "skills" / "neworder" + target.parent.mkdir(parents=True) + target.symlink_to(foreign, target_is_directory=True) + + assert skill_cli.main(["--install", str(tmp_path)]) == 1 + assert target.resolve() == foreign.resolve() + + +def test_install_copies_when_symlinks_unavailable(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: + _no_symlinks(monkeypatch) + assert skill_cli.main(["--install", str(tmp_path)]) == 0 + + target = tmp_path / "skills" / "neworder" + assert not target.is_symlink() + assert (target / "SKILL.md").read_text() == (skill_cli._source_dir() / "SKILL.md").read_text() + + +def test_install_across_windows_drives(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: + # os.path.relpath raises when the package and the project are on different drives + def raise_valueerror(*_args: object, **_kwargs: object) -> str: + raise ValueError("path is on mount 'D:', start on mount 'C:'") + + monkeypatch.setattr(skill_cli.os.path, "relpath", raise_valueerror) + assert skill_cli.main(["--install", str(tmp_path)]) == 0 + + target = tmp_path / "skills" / "neworder" + assert (target / "SKILL.md").read_text() == (skill_cli._source_dir() / "SKILL.md").read_text() + assert skill_cli.main(["--remove", str(tmp_path)]) == 0 + assert not target.exists() + + +def test_install_refreshes_stale_copy(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: + _no_symlinks(monkeypatch) + skill_cli.main(["--install", str(tmp_path)]) + target = tmp_path / "skills" / "neworder" + (target / "SKILL.md").write_text("stale content from an older version") + + assert skill_cli.main(["--install", str(tmp_path)]) == 0 + assert (target / "SKILL.md").read_text() == (skill_cli._source_dir() / "SKILL.md").read_text() + + +def test_remove_deletes_installed_skill(tmp_path: Path) -> None: + skill_cli.main(["--install", str(tmp_path)]) + target = tmp_path / "skills" / "neworder" + assert target.exists() + + assert skill_cli.main(["--remove", str(tmp_path)]) == 0 + assert not target.exists() + assert not target.is_symlink() + + +def test_remove_deletes_copy(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: + _no_symlinks(monkeypatch) + skill_cli.main(["--install", str(tmp_path)]) + target = tmp_path / "skills" / "neworder" + + assert skill_cli.main(["--remove", str(tmp_path)]) == 0 + assert not target.exists() + + +def test_remove_missing_is_noop(tmp_path: Path) -> None: + assert skill_cli.main(["--remove", str(tmp_path)]) == 0 + + +def test_remove_refuses_foreign_directory(tmp_path: Path) -> None: + target = tmp_path / "skills" / "neworder" + target.mkdir(parents=True) + (target / "keepme.txt").write_text("do not delete") + + assert skill_cli.main(["--remove", str(tmp_path)]) == 1 + assert (target / "keepme.txt").read_text() == "do not delete" + + +def test_remove_default_path(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None: + monkeypatch.chdir(tmp_path) + skill_cli.main(["--install"]) + target = tmp_path / ".agents" / "skills" / "neworder" + assert target.exists() + + assert skill_cli.main(["--remove"]) == 0 + assert not target.exists() + + +def test_mutually_exclusive_args_required() -> None: + with pytest.raises(SystemExit): + skill_cli.main([]) + with pytest.raises(SystemExit): + skill_cli.main(["--install", ".", "--remove", "."]) diff --git a/zensical.toml b/zensical.toml index 126c7c0..e77b400 100644 --- a/zensical.toml +++ b/zensical.toml @@ -28,6 +28,7 @@ nav = [ { Infection = "examples/infection.md" }, ]}, {"Tips and Tricks" = "tips.md" }, + {"Agent Skill" = "agent-skill.md" }, {"API Reference" = "api.md" }, { References = "references.md" }, { About = [