stack: docs verify prueft § Commands und § Error contracts in tools/CONTRACT.md getrennt, 10 fehlende Fehlerkontrakt-Zeilen nachgetragen (schliesst #91)

Files changed: - CHANGES.md - VERSION - tools/CONTRACT.md - tools/chemenu/commands/docs_verify.py - tools/chemenu/tests/test_docs_verify.py
2026-09-11 12:19:56 +02:00
parent 441a8151ab
commit 203084477f
5 changed files with 266 additions and 31 deletions
@@ -5,7 +5,10 @@ The wiki's own rule is that a derived copy of recomputable truth must be
 checked or absent. Three such copies survive on purpose because they earn
 their keep as reading material:

-  1. `tools/CONTRACT.md`'s command table (re-derivable from the Typer app)
+  1. `tools/CONTRACT.md`'s two command tables - § Commands and § Error
+     contracts - each re-derivable from the Typer app and checked
+     independently, in both directions, so a row surviving in one table
+     cannot hide its own deletion from the other
  2. the collection and stage contracts (their existence and placement, not
     their content)
  3. the absence of pre-type-system `type: entity` frontmatter in the
@@ -178,6 +181,35 @@ LEGACY_TYPE_RE = re.compile(r"^type:\s*(entity|concept|source|comparison)\s*$",
 # First backticked cell of a markdown table row, e.g. "| `xref add --a ...` | ... |"
 TABLE_CELL_RE = re.compile(r"^\|\s*`([^`]+)`", re.MULTILINE)

+# tools/CONTRACT.md carries two tables whose first cell is a backticked
+# command path - § Commands and § Error contracts - and `check_cli_readme`
+# must not treat them as one pot (Gitea #91): a row deleted from one used to
+# go unnoticed as long as the same name survived in the other, and the
+# second table was not enforced against anything at all.
+COMMANDS_HEADING = "## Commands"
+ERROR_CONTRACTS_HEADING = "## Error contracts"
+
+
+def section_text(full_text: str, heading: str) -> str:
+    """The text of one `##`-level section: from just after `heading`'s own
+    line up to the next `#`- or `##`-level heading, or the end of the
+    document.
+
+    Raises `ValueError` if `heading` is not found verbatim, rather than
+    falling back to scanning the whole document - a renamed heading has to
+    surface as a failure, because silently widening the scope back to
+    "everything" is exactly the bug this function exists to prevent from
+    reappearing under a different name.
+    """
+    pattern = re.compile(
+        r"^" + re.escape(heading) + r"[ \t]*\n(.*?)(?=^#{1,2}[ \t]|\Z)",
+        re.MULTILINE | re.DOTALL,
+    )
+    match = pattern.search(full_text)
+    if match is None:
+        raise ValueError(f"no {heading!r} heading found")
+    return match.group(1)
+

 def registered_commands() -> set[str]:
    """Every command path the CLI exposes, e.g. {'new', 'xref add', ...}.
@@ -213,15 +245,23 @@ def documented_commands(readme_text: str) -> list[str]:


 def check_cli_readme() -> list[str]:
-    """Every registered command must appear in tools/CONTRACT.md, and every
-    command documented there must exist.
+    """Every registered command must appear in tools/CONTRACT.md's own
+    § Commands table, and separately in its § Error contracts table - each
+    direction checked per table, independently of the other.

-    `TABLE_CELL_RE` scans the whole file for any markdown table row whose
-    first cell is backticked - there is no separate "command table" region it
-    is scoped to, so a match in the error-contract table (or any other table
-    shaped the same way) counts too. It reads only the backticked path itself,
-    never the rest of the cell: a changed flag or a rewritten description in
-    an existing row is invisible to this check.
+    The two tables used to be read as one pot: `TABLE_CELL_RE` matched a
+    backticked first cell anywhere in the file, so a row deleted from
+    § Commands went unnoticed as long as the same name still had a row in
+    § Error contracts, and § Error contracts was never itself compared
+    against the registered commands (Gitea #91). `section_text` scopes each
+    table to the text between its own `##` heading and the next one, and
+    raises rather than silently scanning the whole file if a heading has been
+    renamed or removed - a renamed heading must be reported, not read as
+    "table now empty" or "table now everything".
+
+    Within a section, only the first backticked cell of each row is read -
+    a changed flag or a rewritten description in an existing row is invisible
+    to this check, on purpose: it verifies presence, never prose.

    The reverse check matches a documented cell against the full registered
    command path (e.g. `xref add`, `migrate verify`), not just its first
@@ -233,18 +273,36 @@ def check_cli_readme() -> list[str]:
        return [f"{CLI_README.relative_to(config.ROOT)} is missing"]

    text = CLI_README.read_text(encoding="utf-8")
-    cells = documented_commands(text)
-    issues = []
-
    registered = sorted(registered_commands())
-    for command_path in registered:
-        if not any(cell == command_path or cell.startswith(command_path + " ") for cell in cells):
-            issues.append(f"command `{command_path}` is not documented in tools/CONTRACT.md")
+    issues: list[str] = []

-    for cell in cells:
-        if not any(cell == cp or cell.startswith(cp + " ") for cp in registered):
-            first_token = cell.split(" ", 1)[0]
-            issues.append(f"tools/CONTRACT.md documents `{cell}`, but `{first_token}` is not a wikitool command")
+    for heading, label in (
+        (COMMANDS_HEADING, "§ Commands"),
+        (ERROR_CONTRACTS_HEADING, "§ Error contracts"),
+    ):
+        try:
+            section = section_text(text, heading)
+        except ValueError as exc:
+            issues.append(
+                f"tools/CONTRACT.md: {exc} - its {label} table cannot be checked against the CLI"
+            )
+            continue
+
+        cells = documented_commands(section)
+
+        for command_path in registered:
+            if not any(cell == command_path or cell.startswith(command_path + " ") for cell in cells):
+                issues.append(
+                    f"command `{command_path}` is not documented in tools/CONTRACT.md's {label} table"
+                )
+
+        for cell in cells:
+            if not any(cell == cp or cell.startswith(cp + " ") for cp in registered):
+                first_token = cell.split(" ", 1)[0]
+                issues.append(
+                    f"tools/CONTRACT.md's {label} table documents `{cell}`, but `{first_token}` "
+                    "is not a wikitool command"
+                )

    return issues

@@ -42,13 +42,136 @@ def test_invented_subcommand_under_a_real_group_is_caught(tmp_path, monkeypatch)
    group. The reverse check must match the full registered path, not just
    the top-level word."""
    fake = tmp_path / "README.md"
-    fake.write_text("| `xref frobnicate --a X --b Y` | does not exist |\n", encoding="utf-8")
+    fake.write_text(
+        "## Commands\n\n"
+        "| `xref frobnicate --a X --b Y` | does not exist |\n\n"
+        "## Error contracts\n\n"
+        "| `xref frobnicate --a X --b Y` | does not exist |\n",
+        encoding="utf-8",
+    )
    monkeypatch.setattr(docs_verify, "CLI_README", fake)
    monkeypatch.setattr(docs_verify, "registered_commands", lambda: {"xref add", "xref remove"})
    issues = docs_verify.check_cli_readme()
    assert any("xref frobnicate" in issue for issue in issues)


+# --- section-scoped § Commands vs. § Error contracts (Gitea #91) ------------
+
+
+def test_section_text_extracts_between_headings():
+    text = "# T\n\n## A\n\nfoo\n\n## B\n\nbar\n"
+    assert docs_verify.section_text(text, "## A").strip() == "foo"
+
+
+def test_section_text_extends_to_end_of_file_when_last():
+    text = "# T\n\n## A\n\nfoo\nbar\n"
+    assert docs_verify.section_text(text, "## A").strip() == "foo\nbar"
+
+
+def test_section_text_raises_on_missing_heading():
+    with pytest.raises(ValueError):
+        docs_verify.section_text("# T\n\nno headings here\n", "## Commands")
+
+
+def _fake_contract(tmp_path, commands_rows: str, error_rows: str):
+    fake = tmp_path / "CONTRACT.md"
+    fake.write_text(
+        f"## Commands\n\n{commands_rows}\n## Error contracts\n\n{error_rows}\n",
+        encoding="utf-8",
+    )
+    return fake
+
+
+def test_a_row_deleted_from_commands_is_caught_even_if_error_contracts_still_has_it(
+    tmp_path, monkeypatch
+):
+    """Regression for the bug the section split fixes: before, a name
+    surviving in either table hid its own deletion from the other, so
+    § Commands losing a row was invisible as long as § Error contracts still
+    named it."""
+    fake = _fake_contract(
+        tmp_path,
+        commands_rows="| Command | Purpose |\n",  # `frobnicate`'s row was deleted here
+        error_rows="| `frobnicate` | never | yes | retry |\n",
+    )
+    monkeypatch.setattr(docs_verify, "CLI_README", fake)
+    monkeypatch.setattr(docs_verify, "registered_commands", lambda: {"frobnicate"})
+    issues = docs_verify.check_cli_readme()
+    assert any(
+        "frobnicate" in issue and "§ Commands" in issue and "not documented" in issue
+        for issue in issues
+    )
+
+
+def test_error_contracts_is_enforced_against_registered_commands(tmp_path, monkeypatch):
+    """Before the split, § Error contracts was never itself compared against
+    the registered commands - a row missing there was invisible."""
+    fake = _fake_contract(
+        tmp_path,
+        commands_rows="| `frobnicate` | does things |\n",
+        error_rows="| Command | Exit 1 means | Atomic? | Retry policy |\n",
+    )
+    monkeypatch.setattr(docs_verify, "CLI_README", fake)
+    monkeypatch.setattr(docs_verify, "registered_commands", lambda: {"frobnicate"})
+    issues = docs_verify.check_cli_readme()
+    assert any(
+        "frobnicate" in issue and "§ Error contracts" in issue and "not documented" in issue
+        for issue in issues
+    )
+
+
+def test_a_phantom_error_contract_row_is_reported(tmp_path, monkeypatch):
+    """The reverse direction inside § Error contracts: a row for a command
+    that does not exist must be reported there too, not only in § Commands."""
+    fake = _fake_contract(
+        tmp_path,
+        commands_rows="| `frobnicate` | does things |\n",
+        error_rows="| `frobnicate` | never | yes | retry |\n| `ghost command` | never happened | no | n/a |\n",
+    )
+    monkeypatch.setattr(docs_verify, "CLI_README", fake)
+    monkeypatch.setattr(docs_verify, "registered_commands", lambda: {"frobnicate"})
+    issues = docs_verify.check_cli_readme()
+    assert any(
+        "ghost command" in issue and "§ Error contracts" in issue and "not a wikitool command" in issue
+        for issue in issues
+    )
+
+
+def test_a_renamed_commands_heading_is_reported_not_silently_scanned(tmp_path, monkeypatch):
+    """A renamed or removed `## Commands` heading must fail loudly - falling
+    back to scanning the whole file would make the two tables indistinguishable
+    again, which is the exact bug this check exists to prevent."""
+    fake = tmp_path / "CONTRACT.md"
+    fake.write_text(
+        "## Kommandos\n\n"
+        "| `frobnicate` | does things |\n\n"
+        "## Error contracts\n\n"
+        "| `frobnicate` | never | yes | retry |\n",
+        encoding="utf-8",
+    )
+    monkeypatch.setattr(docs_verify, "CLI_README", fake)
+    monkeypatch.setattr(docs_verify, "registered_commands", lambda: {"frobnicate"})
+    issues = docs_verify.check_cli_readme()
+    assert any("no '## Commands' heading found" in issue for issue in issues)
+    assert not any("§ Commands" in issue and "not documented" in issue for issue in issues)
+
+
+def test_a_renamed_error_contracts_heading_is_reported_not_silently_scanned(tmp_path, monkeypatch):
+    fake = tmp_path / "CONTRACT.md"
+    fake.write_text(
+        "## Commands\n\n"
+        "| `frobnicate` | does things |\n\n"
+        "## Fehlerkontrakte\n\n"
+        "| `frobnicate` | never | yes | retry |\n",
+        encoding="utf-8",
+    )
+    monkeypatch.setattr(docs_verify, "CLI_README", fake)
+    monkeypatch.setattr(docs_verify, "registered_commands", lambda: {"frobnicate"})
+    issues = docs_verify.check_cli_readme()
+    assert any("no '## Error contracts' heading found" in issue for issue in issues)
+    assert not any("§ Error contracts" in issue and "not documented" in issue for issue in issues)
+
+
 def test_collection_contracts_exist():
    assert docs_verify.check_collection_contracts() == []