diff --git a/CHANGES.md b/CHANGES.md index d8d3a03..57fc2f0 100644 --- a/CHANGES.md +++ b/CHANGES.md @@ -59,7 +59,7 @@ concern - readable here, never shipped as something to parse. --- -## 7.1.0-beta.28 - 2026-09-26 - gates.md and a run_budget comment name kb pages by title, not by a path that moved +## 7.1.0-beta.30 - 2026-09-26 - new: concept/source record notes name their layout-computed subdirectory; source usage names its required fields (Gitea #150) **Author:** Torben Nehmer @@ -97,8 +97,55 @@ concern - readable here, never shipped as something to parse. - Command records: three more mismatches from #142 aligned to code - docs contract: the merged-stream test pins its own ON FAILURE line - gates.md and a run_budget comment name kb pages by title, not by a path that moved +- cli.py: removed a stale duplicate help-patch line outside the typer._click fallback's try (Gitea #148) +- new: concept/source record notes name their layout-computed subdirectory; source usage names its required fields (Gitea #150) +### new: concept/source record notes name their layout-computed subdirectory; source usage names its required fields (Gitea #150) + +The `new`-command's `cli_contract` record promised `kb/concepts/.md` and +`kb/sources/Source - .md` for the `concept` and `source` variants - flat, with no +subdirectory. Both type-specs have carried a `layout:` for a while (`concept_type`/`source_type` +picks the area, same rule an entity or a project already follows), so the actual path is +`kb/concepts//.md` and `kb/sources//Source - .md`. Nothing wrote a +page to the wrong place - `new` computes the path itself - but the record is exactly what an +agent reads to find one afterwards, and it was wrong: `instructions/gates.md` named two concept +pages by their old flat path (fixed above) with the record itself as the plausible source of that +assumption. Both notes now name the subdirectory and where it comes from; `comparison`'s note was +checked against `types/comparison.md` and left alone; it genuinely has no `layout:`. + +Found in the same pass: `new source`'s example line omitted `source_type` (required, no default +since #66) and `fidelity`/`authority` (enforced by `new` itself since #67), so copying it verbatim +always failed. It now names all three. + +The new test in `tools/chemenu/tests/test_new_page.py` reads each variant's note out of the +record itself and turns it into the path pattern it promises, then checks what `new` actually +wrote against that pattern - coupled to the note text, not to a path re-typed into the test, which +is what let the existing per-type path tests stay green through this exact drift. A companion test +asserts every `Writes \`kb/...\`` variant has a case, so a future variant without one is caught +here instead of silently going unchecked. `tools/CONTRACT.md` regenerated via +`wikitool docs contract --apply`. + +### cli.py: removed a stale duplicate help-patch line outside the typer._click fallback's try (Gitea #148) + +The `typer._click.core.Command.format_help` patch was applied twice: once inside a +`try/except (ImportError, AttributeError)` meant to let a future typer without `typer._click` +degrade to Click's own plain help instead of crashing every invocation, and once more on the next +module-level line, unconditionally. Whenever the `try` actually failed, that second line referenced +two names the failed import never defined and raised `NameError` at import time - the exact crash +the fallback exists to prevent, on every single `wikitool` call. Typer 0.27.2 still has the module, +so nothing showed it in practice; the line was pure dead weight until the day it wasn't. Removed, +so the patch is applied exactly where the `try` already applies it. + +The new regression test drives `chemenu.cli` in a subprocess with a `builtins.__import__` hook +that raises only for `typer._click.core` imported from `chemenu.cli`/`__main__`, then checks that +`wikitool search -h` still exits 0 with Click's own plain help. Two more direct ways to simulate +the missing module were tried and rejected: `sys.modules['typer._click.core'] = None` also breaks +typer's own lazy import of `typer._click.decorators` inside `get_help_option`, so `-h` fails +regardless of what `cli.py` does; `importlib.reload(chemenu.cli)` reruns the module in the same +`__dict__`, so the names from the first, real import survive and the broken line runs +successfully - a test built that way would stay green against the exact bug it exists to catch. + ### gates.md and a run_budget comment name kb pages by title, not by a path that moved `instructions/gates.md` pointed at `kb/concepts/Mass-Update Gate.md` and diff --git a/VERSION b/VERSION index 9640530..cbc4b30 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -7.1.0-beta.28 +7.1.0-beta.30 diff --git a/tools/CONTRACT.md b/tools/CONTRACT.md index a8630e9..9b75816 100644 --- a/tools/CONTRACT.md +++ b/tools/CONTRACT.md @@ -155,9 +155,9 @@ Scaffold a new wiki page of any type. **SYNOPSIS** - `wikitool new --name "" [--type ] [--set field=value ...]` - Any type; its type-spec decides fields, directory, title prefix and template. -- `wikitool new entity --name "" --set entity_type= [--set tags=a,b] [--set related=X,Y] [--set sources="Source - Z"] [--set provenance=sourced|general|mixed]` - Writes `kb/entities//.md` -- `wikitool new concept --name "" --set concept_type= ...` - Writes `kb/concepts/.md` -- `wikitool new source --name "" --set raw_files=raw/notes/x.md,raw/notes/y.md [--set source_url=] [--set entities=A,B] [--set concepts=C,D]` - Writes `kb/sources/Source - .md` (prefix added automatically) with a `raw_files:` list; rejects paths that don't exist +- `wikitool new entity --name "" --set entity_type= [--set tags=a,b] [--set related=X,Y] [--set sources="Source - Z"] [--set provenance=sourced|general|mixed]` - Writes `kb/entities//.md` - `` from `entity_type` via the type-spec's `layout:` +- `wikitool new concept --name "" --set concept_type= ...` - Writes `kb/concepts//.md` - `` from `concept_type` via the type-spec's `layout:` +- `wikitool new source --name "" --set source_type= --set raw_files=raw/notes/x.md,raw/notes/y.md --set fidelity= --set authority= [--set source_url=] [--set entities=A,B] [--set concepts=C,D]` - Writes `kb/sources//Source - .md` (prefix added automatically; `` from `source_type` via the type-spec's `layout:`) with a `raw_files:` list; rejects paths that don't exist - `wikitool new comparison --name "X vs Y" --set entities=X,Y` - Writes `kb/comparisons/X vs Y.md` - `wikitool new project --name "" --set responsibility= [--resume]` - Writes `kb/gtd//.md` and, with a task tracker configured, a same-named tracker project diff --git a/tools/chemenu/cli.py b/tools/chemenu/cli.py index ed79414..33903e4 100644 --- a/tools/chemenu/cli.py +++ b/tools/chemenu/cli.py @@ -262,9 +262,6 @@ except (ImportError, AttributeError): pass -_typer_click_core.Command.format_help = _contract_format_help - - def main() -> None: # Iteration Budget Gate / Loop-Breaker (see instructions/gates.md # "Iteration Budget Gate and loop-breaker"): recorded and enforced here, diff --git a/tools/chemenu/commands/new_page.py b/tools/chemenu/commands/new_page.py index 705e7bc..54c80f5 100644 --- a/tools/chemenu/commands/new_page.py +++ b/tools/chemenu/commands/new_page.py @@ -337,17 +337,21 @@ def _ensure_tracker_project(page_title: str, *, resume: bool) -> Optional[str]: usage='new entity --name "" --set entity_type= [--set tags=a,b] ' "[--set related=X,Y] [--set sources=\"Source - Z\"] " "[--set provenance=sourced|general|mixed]", - notes="Writes `kb/entities//.md`", + notes="Writes `kb/entities//.md` - `` from `entity_type` " + "via the type-spec's `layout:`", ), cli_contract.Variant( usage='new concept --name "" --set concept_type= ...', - notes="Writes `kb/concepts/.md`", + notes="Writes `kb/concepts//.md` - `` from `concept_type` " + "via the type-spec's `layout:`", ), cli_contract.Variant( - usage='new source --name "" --set raw_files=raw/notes/x.md,raw/notes/y.md ' + usage='new source --name "" --set source_type= ' + "--set raw_files=raw/notes/x.md,raw/notes/y.md --set fidelity= --set authority= " "[--set source_url=] [--set entities=A,B] [--set concepts=C,D]", - notes="Writes `kb/sources/Source - .md` (prefix added automatically) with a " - "`raw_files:` list; rejects paths that don't exist", + notes="Writes `kb/sources//Source - .md` (prefix added automatically; " + "`` from `source_type` via the type-spec's `layout:`) with a `raw_files:` " + "list; rejects paths that don't exist", ), cli_contract.Variant( usage='new comparison --name "X vs Y" --set entities=X,Y', diff --git a/tools/chemenu/tests/test_cli.py b/tools/chemenu/tests/test_cli.py index b01d25a..5a670d9 100644 --- a/tools/chemenu/tests/test_cli.py +++ b/tools/chemenu/tests/test_cli.py @@ -266,6 +266,55 @@ def test_a_usage_error_is_unframed_and_names_wikitool(monkeypatch): assert not FRAME_CHARS & set(text) +# --- the `typer._click` fallback actually falls back (Gitea #148) --- +# +# `cli.py`'s help patch is wrapped in `try/except (ImportError, AttributeError)` +# specifically so a future typer without `typer._click.core` degrades to plain +# Click help instead of crashing every invocation. A subprocess with a targeted +# import hook is what actually exercises that: `sys.modules[...] = None` was +# tried and rejected, because typer itself lazily imports `typer._click.decorators` +# from inside `get_help_option` - poisoning the module that way breaks typer's own +# `-h` handling regardless of what `cli.py` does, on both the buggy and the fixed +# code. `importlib.reload(chemenu.cli)` was tried and rejected too: reload runs +# in the same module `__dict__`, so `_typer_click_core`/`_contract_format_help` +# from the first (real) import survive and the broken module-level line at the +# end runs successfully - a test built that way would stay green against the bug +# it exists to catch. +_IMPORT_HOOK = """ +import builtins, runpy, sys +_real_import = builtins.__import__ +def _guarded(name, globals=None, locals=None, fromlist=(), level=0): + if name == "typer._click.core" and (globals or {}).get("__name__") in ("chemenu.cli", "__main__"): + raise ImportError("simulated: typer without _click, for Gitea #148") + return _real_import(name, globals, locals, fromlist, level) +builtins.__import__ = _guarded +sys.argv = ["wikitool", "search", "-h"] +runpy.run_module("chemenu.cli", run_name="__main__") +""" + + +def test_missing_typer_click_falls_back_to_plain_click_help_instead_of_crashing(tmp_path): + tools_dir = Path(__file__).resolve().parents[2] + env = dict(os.environ) + env["CHEMENU_ROOT"] = str(tmp_path) + env["WIKI_TRACE_DIR"] = str(tmp_path / "trace") + env["WIKITOOL_SESSION_ID"] = "test-148-typer-click-fallback" + result = subprocess.run( + [sys.executable, "-c", _IMPORT_HOOK], + cwd=str(tools_dir), + env=env, + capture_output=True, + text=True, + timeout=30, + ) + assert result.returncode == 0, result.stdout + result.stderr + assert "Traceback" not in result.stderr + assert result.stdout.startswith("Usage:") + # Click's own plain help, not the cli_contract-rendered record. + assert "SYNOPSIS" not in result.stdout + assert "EXIT STATUS" not in result.stdout + + def test_every_record_shows_at_least_one_example(): """Every real command record carries a copyable example (Gitea #142).""" missing = [path for path, rec in cli_contract.all_records().items() if not rec.examples] diff --git a/tools/chemenu/tests/test_new_page.py b/tools/chemenu/tests/test_new_page.py index a2ab5ae..79d31b0 100644 --- a/tools/chemenu/tests/test_new_page.py +++ b/tools/chemenu/tests/test_new_page.py @@ -1,9 +1,13 @@ import contextlib import http.server import json +import re import threading from pathlib import Path +import pytest + +from chemenu import cli_contract from chemenu.commands._util import coerce_set_value, parse_set_fields from chemenu.commands.new_page import _page_subdir from chemenu.frontmatter_io import read_page @@ -690,6 +694,132 @@ def test_new_comparison_rejects_single_entity(monkeypatch, kb_dir): assert result.exit_code != 0 +# --- the `new` record's own path notes match what `new` actually writes +# (Gitea #150) --- +# +# Coupled to the record's notes text, not to a path hand-copied into the +# test - the existing per-type tests above (e.g. +# `test_new_concept_creates_page_in_its_subtype_area`) assert a path against +# the *code*, so a note that drifted out of sync with the code would still +# pass every one of them. This reads the note itself and turns it into the +# pattern it promises, so a drift like #150's (the `concept`/`source` notes +# lost their `` when `layout:` was added to their type-specs) fails +# here even if every code-level path test stays green. + +def _path_pattern_from_note(notes: str, name: str) -> re.Pattern: + """The first backtick-quoted span in a `new` variant's notes is its path + template. `` becomes the literal name under test; any other + `<...>` placeholder (a type-spec-computed subdirectory) becomes a + wildcard segment. A note with no placeholder at all (`comparison`, which + genuinely has none) is matched as the literal path it names. The leading + `kb/` is stripped: the actual path under test is already relative to + `kb_dir`, which *is* that root.""" + template = re.search(r"`([^`]+)`", notes).group(1) + assert template.startswith("kb/"), template + template = template[len("kb/"):] + pattern = "".join( + re.escape(name) if part == "" + else r"[^/]+" if re.fullmatch(r"<[^>]+>", part) + else re.escape(part) + for part in re.split(r"(<[^>]+>)", template) + ) + return re.compile(pattern + r"\Z") + + +_NEW_PATH_VARIANTS = [ + v for v in cli_contract.get("new").synopsis if v.notes.startswith("Writes `kb/") +] + + +def _new_variant_notes(usage_prefix: str) -> str: + for variant in _NEW_PATH_VARIANTS: + if variant.usage.startswith(usage_prefix): + return variant.notes + raise AssertionError(f"no `new` record variant starts with {usage_prefix!r}") + + +def test_every_writes_kb_variant_has_a_path_case(): + """A future variant whose notes promise a `kb/` path but that nobody + parametrized below would otherwise go unchecked - same failure shape as + #150 itself, one level up.""" + covered_prefixes = ("new entity ", "new concept ", "new source ", "new comparison ", "new project ") + uncovered = [ + v.usage for v in _NEW_PATH_VARIANTS + if not any(v.usage.startswith(p) for p in covered_prefixes) + ] + assert uncovered == [] + + +def test_new_entity_path_matches_its_record_note(monkeypatch, kb_dir): + name = "Widget" + result = _invoke_new(monkeypatch, kb_dir, [ + "new", "entity", "--name", name, "--set", "entity_type=tool", + ]) + assert result.exit_code == 0, result.output + pattern = _path_pattern_from_note(_new_variant_notes("new entity "), name) + written = next(kb_dir.glob(f"entities/**/{name}.md")) + assert pattern.match(str(written.relative_to(kb_dir))) + + +def test_new_concept_path_matches_its_record_note(monkeypatch, kb_dir): + name = "Event Sourcing" + result = _invoke_new(monkeypatch, kb_dir, [ + "new", "concept", "--name", name, "--set", "concept_type=pattern", + ]) + assert result.exit_code == 0, result.output + pattern = _path_pattern_from_note(_new_variant_notes("new concept "), name) + written = next(kb_dir.glob(f"concepts/**/{name}.md")) + assert pattern.match(str(written.relative_to(kb_dir))) + + +def test_new_source_path_matches_its_record_note_and_its_usage_is_runnable(monkeypatch, kb_dir): + """Also #150's second finding: the `usage` line itself omitted + `source_type`/`fidelity`/`authority` and could never succeed as written. + Running the corrected `usage` verbatim (substituting ``/``/etc.) + is the test that it now can.""" + monkeypatch.setenv("WIKI_AUTHOR", "Torben") + name = "gateway.example.net" + _fixture_raw_file(monkeypatch, kb_dir, "raw/notes/gateway.example.net.md") + result = _invoke_new(monkeypatch, kb_dir, [ + "new", "source", "--name", name, + "--set", "source_type=notes", + "--set", "raw_files=raw/notes/gateway.example.net.md", + "--set", "fidelity=verbatim", "--set", "authority=reporting", + ]) + assert result.exit_code == 0, result.output + pattern = _path_pattern_from_note(_new_variant_notes("new source "), name) + written = next(kb_dir.glob(f"sources/**/Source - {name}.md")) + assert pattern.match(str(written.relative_to(kb_dir))) + + +def test_new_comparison_path_matches_its_record_note(monkeypatch, kb_dir): + """`comparison` has no `layout:`, so its note names a literal path with + no placeholder - the name used here is exactly what that literal names, + which is what makes this case a genuine check rather than a tautology: + a `layout:` added to `types/comparison.md` without updating the note + would make the literal not match the now-nested actual path.""" + name = "X vs Y" + result = _invoke_new(monkeypatch, kb_dir, [ + "new", "comparison", "--name", name, "--set", "entities=aurora,Borealis", + ]) + assert result.exit_code == 0, result.output + pattern = _path_pattern_from_note(_new_variant_notes("new comparison "), name) + written = kb_dir / "comparisons" / f"{name}.md" + assert written.exists() + assert pattern.match(str(written.relative_to(kb_dir))) + + +def test_new_project_path_matches_its_record_note(monkeypatch, kb_dir): + name = "Testvorhaben" + result = _invoke_new(monkeypatch, kb_dir, [ + "new", "project", "--name", name, "--set", "responsibility=haus", + ]) + assert result.exit_code == 0, result.output + pattern = _path_pattern_from_note(_new_variant_notes("new project "), name) + written = next(kb_dir.glob(f"gtd/**/{name}.md")) + assert pattern.match(str(written.relative_to(kb_dir))) + + def test_repeated_set_appends_for_array_fields(): """The separator-free way to pass an element containing a comma. The schema type decides: only array fields append."""