Skip to content

Fix the known issues: validate checks generated code, Click backend parity, reproducible output, no-extras support - #59

Merged
jaykv merged 23 commits into
mainfrom
fix/known-issues
Oct 12, 2026
Merged

jaykv merged 23 commits into
mainfrom
fix/known-issues

Conversation

@jaykv

@jaykv jaykv commented Oct 12, 2026

Copy link
Copy Markdown
Owner

Stack 8 of 8. Base: feat/build-targets. Merge in order; each PR shows only its own changes.

Summary

Fixes the bugs that the audits of 0.7.0 found but could not fix in scope. Each fix has a regression test that was written first.

  • cli validate now compiles and imports the generated code.
  • The Click backend follows the documented meaning of params and supports types and config.
  • Generated code is reproducible, and examples/generated is checked against the generator by a test.
  • cliffy lints, type-checks and passes its tests without the optional extras; a CI job covers that.
  • Every error found while code is generated is reported the same way by every command, as ~ error in <file>: command <name> ..., with no traceback.
  • Running the test suite no longer extracts zipapps into ~/.shiv.

Bugs fixed

Bug Fix
The test suite extracted zipapps into ~/.shiv and never cleaned up A session fixture points SHIV_ROOT at the pytest temp directory; the docs snippet runner does the same
The cli init --raw template could not run its own tests Both templates are tested end to end; the raw template gets one command and one test
cli validate passed manifests whose code did not import It compiles and imports the code and builds the Typer command tree; errors name the command
cli --version printed rich-click's version The version is passed explicitly
manifestVersion was never checked v1, v2 and unknown values are errors in manifests, includes and blueprint specs; examples fixed
cli test counter could print "10 of 9"; $ and > were swapped in a message; results differed on CI The counter counts test cases only; message corrected; assertions see output without terminal styling
Click backend: explicit params were always options; types and config were ignored Parity with Typer; clear errors for Typer-only definitions; rich-click is optional
"Greet\nuser" became "Greetuser" A line break becomes a space; paragraphs are kept
Name collisions (db-app and group db) and a var named my-var broke the generated module Collision-free names; invalid var names are rejected
Non-string vars were rejected Numbers and booleans are allowed and keep their type; schema regenerated
Vars were resolved in one pass Transitive resolution with cycle detection
A Jinja error in a comment gave a traceback Reported with file, line and text; comments stay part of the template (documented)
Corrupt metadata was listed as <name>.json, so cli rm could not remove it Listed as <name>
Generated code started with a timestamp Stable header; examples regenerated; make generate-clis only renders
Wrong field descriptions in the manifest models Fixed at the source; the overrides in scripts/docs.py are gone
Installs without extras failed tests and mypy Generated Click code falls back to plain click, cliffy/rich.py type-checks either way, the ai tests use importorskip, and a no-extras CI job is added
cli dev ignored the blueprint file It watches the blueprint file and included manifests

Three reported bugs (inline Typer definitions in shorthand params, global_params on commands without params, include resolution order) were already fixed lower in the stack and have tests there.

Breaking changes

  1. cli validate imports the generated code, so imports, functions and {expression} vars run during validation.
  2. manifestVersion other than v3 is an error.
  3. vars keys must be Python identifiers that the generated code does not use.
  4. A self-referencing or circular var is an error; it used to stay as literal {{ }} text.
  5. Click: an explicit param without -- is a positional argument (it was an option), matching Typer and the docs.
  6. Click: a shorthand argument without ! is optional.
  7. Click: config is applied; rich_help_panel becomes rich-click's panel and then requires rich-click.
  8. Click: Typer definitions (in types, inline, or as a signature string) are rejected with a manifest-level error.
  9. Both backends: a plain type in types is an alias. --n: N with N: int is an option; an explicit param keeps its default and help.
  10. The first line of generated code is ## Generated <name> with cliffy <version>.
  11. Generated Click code imports rich-click with a fallback to click; cli build ships rich-click and does not pin click separately.
  12. Help text: line breaks become spaces instead of being removed.
  13. Generation errors exit with ~ error in <file>: ... instead of raising ValueError.
  14. cli test strips terminal styling before running a manifest's assertions.
  15. cli init --raw output has a command and a test.
  16. Makefile: generate-clis renders with scripts/examples.py; generate-cleanup is removed.
  17. A corrupt metadata entry is named without .json.

All of these are in docs/project/changelog.md and docs/project/migration.md.

Integration with the build targets PR

This branch was developed in parallel and merged onto feat/build-targets. The points where the two met:

  • One error path. A pybash or Python syntax error in a run block gives the same manifest-level message (command, line, column) from every command, cli validate included. A test asserts validate and render print the identical message once.
  • Builder. It ships the new stable header unchanged, and detects the framework from the generated import (rich-click, typer or click) explicitly.
  • Tests that pinned old behaviour were updated where the behaviour is one of the bugs above (help newlines, Click positional arguments, metadata name, ValueError versus a reported error). No test was deleted, skipped or xfailed, apart from the importorskip for the ai tests when the extra is absent.

How verified

  • Python 3.11 and 3.15, fresh environments from the lock: ruff format --check, ruff check, mypy cliffy --strict, mypy tests, pytest -n auto (722 passed, 1 skipped: the opt-in native build), the same with GITHUB_ACTIONS=true, make docs-check, mkdocs build --strict.
  • Without extras (uv sync --locked, Python 3.11): lint and mypy clean; 670 passed, 2 skipped (the ai tests and the native build).

Known issues left

Listed in docs/project/known-issues.md:

  • cli load, render and build do not import the generated code; an unknown type name is only found by validate and test.
  • cli test does not check the exit status of shell steps; a missing program ends the run with a traceback.
  • cli init <name> without --raw rejects names with a hyphen.
  • Includes fall back to the working directory.
  • A full Typer definition in types replaces the param's own default, help and short.

Not verified: Click argument help= without rich-click on click older than 8.5 (the lock has 8.5.0; pyproject.toml allows >=8.2.1).

🤖 Generated with Claude Code

jaykv and others added 23 commits October 12, 2026 12:00
Zipapps built by the tests and by the docs snippet check unpack themselves into
SHIV_ROOT when they run, which defaults to ~/.shiv and is never cleaned up.
A session fixture now points SHIV_ROOT at the pytest temp directory for every
test, subprocess and xdist worker, and scripts/docs.py does the same for the
snippet runner.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`cli init --raw` wrote a manifest with no commands and no tests, so there was
nothing to run and `cli test` exited with an error. Both templates are now
checked end to end: validate, run, and pass their own tests.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A root command and a group could map to the same Python name (command
`db-app` and group `db` are both `db_app`), as could two groups, a command
and its alias copy, or a command and a name from `vars`, `imports` or
`functions`. The commander now reserves every name it emits and appends a
numeric suffix to a name that is taken.

A `vars` key that is not a Python identifier (`my-var`), is a keyword, or is a
name the generated module defines itself is rejected with a manifest-level
error instead of producing a SyntaxError in the generated code.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A manifest passed validation even when the code generated from it was not
valid Python or failed on import. `cli validate` now compiles the generated
module and imports it (which defines the commands without running them) and
reports a failure against the part of the manifest it came from, for example
`commands.db.create: NameError: name 'N' is not defined`.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
With the rich extra installed, the version lookup of `click.version_option()`
ran inside rich-click and reported rich-click's version.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The key was removed from the manifest before validation, so the version check
never ran and `manifestVersion: v1` loaded as if it was v3. The version is now
checked for every file the transformer reads (manifests, includes and blueprint
specs) and the error says what to change. examples/town.yaml and pydev.yaml
declared v1 and now declare v3.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Shell steps advanced the counter too, so it could print "10 of 9". The
message for a shell step without a prefix also described `$` and `>` the wrong
way round: `$` runs the command directly, `>` runs it through a shell.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Newlines were removed from help text without a replacement, so "Greet\nuser"
became "Greetuser". A line break is now a space and a blank line stays a
paragraph break, for commands and groups in both backends.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…nfig

With `use_click: true`:
- an explicit param whose name has no `--` is a positional argument (it used to
  become an option), and argument help is passed on
- a shorthand argument without `!` is optional, as it is for Typer
- `types` are resolved: a plain type (`N: int`, `N: int = 5`) or a complete
  Click decorator. A Typer definition, a Typer signature string or an inline
  `typer.Option(...)` is rejected with an error that names the command
- `Optional[X]`, `X | None` and `list[X]` are translated to Click types
- command and template `config` is passed to `command()`; `rich_help_panel`
  becomes rich-click's `panel`
- a help-only subcommand no longer generates an empty function

For both backends a plain type in `types` is now an alias: the param keeps the
kind its name gives it (`--n` is an option) and its own default and help. The
Typer backend used to replace the whole definition, which turned `--n: N` into
a positional argument and dropped the help of an explicit param.

Code generation errors raised for a command are prefixed with `commands.<name>`.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…line

- A var that uses another computed var is now resolved, whatever order the
  vars are written in. A circular reference is an error that names the circle.
- `vars` values can be numbers and booleans. They keep their type in `{{ }}`
  expressions and in the generated Python variable. A `{expression}` var with
  more than one key is rejected instead of silently dropping the rest.
  examples/cliffy_schema.json is regenerated and checked by a test.
- A Jinja error is reported as manifest file, line and line text instead of a
  traceback. Comments stay part of the template on purpose: Jinja statements
  are written in comments so that the file stays valid YAML. The message says
  so and shows how to keep literal braces.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
It was listed as `<name>.json`, which `cli rm` and `cli rm-all` could not find,
so the entry could not be removed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The first line of the generated code held the time of generation, so two
renders of the same manifest never matched. It now names the CLI and the cliffy
version only.

examples/generated/*.py are regenerated from the example manifests by
scripts/examples.py (`make generate-clis`), which only renders: nothing is
installed or loaded, so the generate-cleanup target and GENERATE_DEPS are gone.
venv.py had no manifest and is removed, callbacks.py was missing. A test
compares every file with a fresh render of its manifest.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The descriptions in cliffy/manifest.py feed the JSON schema, the `cli init`
template and the manifest reference. Several were wrong: a missing space
("CLI.Supports"), `vars` mentioning `${some_env_var}`, `includes` describing a
merge order that is not what happens, `rich_help_panel` naming a `@rich_help`
decorator that does not exist. They are fixed in the models and
FIELD_OVERRIDES in scripts/docs.py, which papered over them for the docs only,
is removed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Only the manifest itself was watched, so editing the Python file of a file
blueprint (or an included manifest) did not reload the CLI. The watcher now
follows every file the CLI is generated from and re-reads that list on each
change, so a blueprint or include added while it runs is picked up too.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- Generated Click code falls back to plain Click when rich-click is not
  installed, so `use_click: true` no longer needs the `rich` extra. A manifest
  that uses `rich_help_panel` still imports rich-click without a fallback.
- cliffy/rich.py type-checks the same way with and without rich-click: for the
  type checker `click` is always Click. The pydantic-ai output validator is
  registered with a call, so `mypy --strict` passes without the `ai` extra.
- tests/test_ai.py is skipped with `pytest.importorskip` when pydantic-ai is
  absent. What cliffy does without the extra is tested in test_ai_extra.py,
  which needs no extra.
- CI gets a job that syncs without extras and runs lint and tests.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Guides, reference notes, the changelog and the migration notes describe the
new behavior of validate, manifestVersion, vars, the Click backend, includes,
cli test and cli dev. Known issues lists what still reproduces, including what
was found while fixing. The Click snippet has tests now, and the vars snippet
shows a number and a var built from other vars. Reference pages regenerated.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
It left an empty nested/directory behind in the working directory.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
From Python 3.14 on annotations are evaluated lazily, so a param with an
unknown type no longer fails when the generated module is imported. Typer
reads the signatures when the CLI starts; validate now does the same.
README: describe validate and the rich extra as they are now.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Four tests assumed that rich-click is installed: three builder tests read its
version to build the expected pin, and a requirements test used it as its
example of a package with an underscore. Without extras the builder requests
rich-click without a pin, and the tests now expect exactly that.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
rich colours help output when GITHUB_ACTIONS is set, also when it is captured,
so the option names were not found in the raw output on a CI runner.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
rich styles help output when GITHUB_ACTIONS is set, even when it is captured.
An assertion such as `'--verbose' in result.output` then passed locally and
failed on a CI runner, as docs/snippets/templates.yaml did under docs-check.
`cli test` now gives the assertions the unstyled text.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Conflicts
- cliffy/cli.py: both import lists kept (build targets and the generated-code
  checker). In `cli test` the shell-step SyntaxError report of this side and
  the test-case-only counter of the other side are both kept.
- cliffy/commander.py, commanders/typer.py, commanders/click.py: import lists
  combined (re + ast, callback_label + clean_help); the stable header replaces
  the timestamp line on both commanders.
- cliffy/tester.py: pybash 0.4 API for shell steps plus the unstyled output
  for assertions.
- docs/project/migration.md: the table of manifest changes is kept below the
  new sections on shell quoting and `cli build` output.
- tests/test_callbacks.py: same fix on both sides, one comment kept.

Semantic integration
- One error path. A problem found while code is generated stops every
  command, validate included, with `~ error in <file>: ...` and no traceback.
  A ScriptError keeps its own label (command, block, line, column) and is not
  wrapped a second time; other generation errors are labelled
  `command `<name>`: ...`. The checker behind `cli validate` uses the same
  labels and now only reports what shows when the code is imported.
- Header. Generated code starts with `## Generated <name> with cliffy
  <version>`, which the builder ships as it is. The builder still strips the
  timestamp of a header written by cliffy 0.6, for CLIs loaded back then.
- Framework detection in the builder is explicit: an import at any
  indentation counts, and click is only a requirement of its own when neither
  typer nor rich-click is imported. Generated Click code therefore uses the
  ordinary four-line try/except fallback and is built with rich-click.
- examples/generated, the JSON schema and the reference pages are regenerated
  with the merged generator. Known issues: tracebacks for generation errors
  and dropped untokenizable run blocks no longer reproduce and are removed.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The test read the version of rich-click, which is an optional extra. Without
it the builder requests rich-click without a pin, and the test now expects
that.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@sourcery-ai

sourcery-ai Bot commented Oct 12, 2026

Copy link
Copy Markdown

Sorry @jaykv, your pull request is larger than the review limit of 150,000 diff characters

@coderabbitai

coderabbitai Bot commented Oct 12, 2026

Copy link
Copy Markdown
Contributor

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration
  • Configuration used: Repository UI
  • Review profile: CHILL
  • Plan: Advanced
  • Run ID: e65205aa-8b49-4c7b-b736-270e7f816416

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@jaykv
jaykv changed the base branch from feat/build-targets to main October 12, 2026 21:23
@jaykv
jaykv merged commit 0129799 into main Oct 12, 2026
8 checks passed
@jaykv
jaykv deleted the fix/known-issues branch October 12, 2026 21:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant