Skip to content

Docs/skill review + usability fixes found while documenting - #99

Merged
yannrichet merged 2 commits into
mainfrom
claude/fz-docs-skills-review-8xkpgw
Oct 1, 2026
Merged

yannrichet merged 2 commits into
mainfrom
claude/fz-docs-skills-review-8xkpgw

Conversation

@yannrichet

@yannrichet yannrichet commented Oct 1, 2026 •

Copy link
Copy Markdown
Member

Pull Request

Description

Two commits:

  1. Documentation and agent-skill review (03a5b6d), rebased on the doc/ restructuring of P0-8 sh:// path fix + P0-1/P0-3/P0-4/P0-5 follow-ups, README reduction (P1-5) #98 and the sh:// fix P0-8. Every finding was checked by running fz on current main.
  2. Code fixes for the defects that review uncovered (086be68), with tests and docs.

Related Issues

Relates to #98 (doc restructuring), P0-8 (sh:// path resolution).

Type of Change

  • Bug fix (non-breaking change which fixes an issue)
  • Documentation update
  • Test addition/modification
  • Breaking change — see "Behavior changes" below; none breaks a model that sets delim

Changes Made

Code fixes (086be68)

Defect (verified on main) Fix
timeout=0 / FZ_RUN_TIMEOUT=0 made every case time out immediately (only a model "timeout": 0 disabled it) 0 means "no timeout" at every level (fz/runners/manager.py); a negative timeout= raises ValueError
A model without delim only recognized $(x) (${x} silently left in compiled files) while the CLI without --model only recognized ${x} Without delim/var_delim, both $(x) and ${x} are recognized (DEFAULT_VAR_DELIM = "(){}", get_var_delim(), _delim_pairs() in fz/interpreter.py); the CLI default model no longer forces {}
fzr(path, vars, model, "sh://bash run.sh") created a directory named sh:/bash run.sh and ran every case without calculator fzr() raises ValueError when results_dir looks like a URI
fz list showed aliases by uri and --check reported installed-wrapper aliases ({"uri": "sh://", "models": {...}}) as failed (Empty sh:// command); a global alias overwrote a project one with the same name Aliases listed by file name with path/uri; --check validates each models command; project shadows global; fzl and fz list share _print_fzl_result()
fz install model X --global wrappers failed outside the install directory (bash .fz/calculators/X.sh resolved against the launch dir) .fz/... paths of an alias are anchored to the .fz/ it was loaded from (_anchor_fz_paths() in fz/helpers.py)
One empty .fz/tmp/fz_temp_* directory left per run Empty directories removed after the run; leftover files still kept

Not changed on purpose: a case without parsable outputs keeps status="done" (the reason is in error). I first changed it to failed, then reverted it: tests/test_examples_advanced.py::test_non_numeric_variables states this is deliberate ("do not expect failed, as it is a failure due to wrong input, not calculation failure"). The doc now says so explicitly.

Behavior changes to note

  • A model without delim now also reads ${...} as variables: a template containing other ${...} text (shell snippets) with such a model will report those names. Models that set delim are unchanged. Documented in NEWS.md.
  • fz list --format json: calculator keys are alias names (previously URIs) and gain path/uri.
  • timeout=0 / FZ_RUN_TIMEOUT=0 change meaning from "immediate timeout" to "no timeout".

Documentation (03a5b6d, updated by 086be68)

  • New doc/limitations.md: constraints and pitfalls checked by running fz.
  • 106 fz.fzr(...) examples in doc/ and examples/ passed the calculator positionally; they now use keywords.
  • Documented as they behave: no ?var conversion (notebook 02 fixed), fzc per-case sub-directories, first Ctrl+C terminates running cases, cache://_, sh:// argument appending, SSH auth/host keys, funz:// UDP port, fzd has no --format, DataFrame designs Python-only, config read at import (reload_config()), fz.shell imports.
  • skills/fz/, /fz:run, NEWS.md updated.

Testing Performed

Automated Tests

  • New tests/test_usability_fixes.py (14 tests: each fix, plus the explicit-delim and missing-output cases)
  • Full CI selection (ci.yml ignore list), FZ_SKILL_E2E=0: 1055 passed, 92 skipped, 5 failed — the same 5 fail on main in this environment (permission tests run as root, local yq version): test_fzi_with_permission_denied, test_install_model_with_permission_error, test_fzc_with_readonly_output_directory, test_evaluate_yq_scalar, test_evaluate_yq_prefix_detection_and_stripping
  • test_cli_commands.py, test_examples_*.py, test_interrupt_handling.py (-m "not slow"): 71 passed, 7 skipped (same as main)
  • flake8 fz/ --select=E9,F63,F7,F82: 0
  • test_skill_e2e.py not run on the branch (runs real Claude sessions)

Manual Testing

  • Linux: each fix reproduced before/after with scratch scripts; --global checked with the real fz-Modelica wrapper installed into a separate HOME

Additional Notes

The companion site PR is Funz/fz.github.io#3 (documents the fixed behavior, with notes for fz ≤ 1.2). It links to doc/limitations.md, so this PR should be merged first.

🤖 Generated with Claude Code

https://claude.ai/code/session_012GxLbauyVHBQPCeSow8hdh

…uctured doc/

Re-applies the unmerged review of claude/fz-docs-skills-review-8xkpgw on
top of #98 (doc/ restructuring) and P0-8 (sh:// path resolution), after
re-checking every finding against current main by running fz:

- doc/limitations.md (new): verified constraints and pitfalls; sh:// part
  updated for P0-8 (argument appending remains a trap)
- fzr examples passing calculators as 4th positional argument (results_dir)
- default delimiters (() for variables without delim), no ?var conversion
- fzc per-case sub-directories, fzo on case dirs; skill ladder fixed
- FZ_RUN_TIMEOUT=0, first Ctrl+C terminates running cases, cache://_
- os.environ after import -> reload_config(); fz.shell imports
- funz:// UDP port, SSH auth/host keys, fz list and --global caveats
- notebook 02: ?(name) needs varprefix '?'

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012GxLbauyVHBQPCeSow8hdh
- timeout=0 / FZ_RUN_TIMEOUT=0 mean no timeout (they timed every case out
  immediately); negative timeout= raises ValueError
- a model without delim/var_delim recognizes both $(x) and ${x}; the CLI
  without --model uses the same default (Python only knew $(x), the CLI
  only ${x})
- fzr() raises ValueError when results_dir looks like a calculator URI
  (calculator passed as 4th positional argument)
- fzl / fz list: aliases listed by file name with uri and path; --check
  validates each command of an alias's models map (installed-wrapper
  aliases no longer fail with 'Empty sh:// command'); project aliases
  shadow global ones; fzl and fz list share their output code
- .fz/... paths of calculator aliases are resolved against the .fz/ they
  were loaded from, so 'fz install --global' wrappers run anywhere
- empty .fz/tmp/fz_temp_* directories are removed after a run

Status of a case without parsable outputs stays 'done' (deliberate, see
test_examples_advanced.test_non_numeric_variables); documented as such.
Docs (doc/, skill, NEWS) updated; tests/test_usability_fixes.py added.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012GxLbauyVHBQPCeSow8hdh
yannrichet pushed a commit to Funz/fz.github.io that referenced this pull request Oct 1, 2026
timeout 0, default delimiters, fzr results_dir guard, fz list output and
checks, global-install runner paths, .fz/tmp cleanup; fz <= 1.2 behavior
noted where users of the released version are affected.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_012GxLbauyVHBQPCeSow8hdh
@yannrichet yannrichet changed the title docs/skill: constraints page and corrections verified against the code Docs/skill review + usability fixes found while documenting Oct 1, 2026
@yannrichet
yannrichet merged commit 8b207d6 into main Oct 1, 2026
43 checks passed
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.

2 participants