feat: wikitool move - eine kb-Seite folgt ihrem Subtype ins berechnete Verzeichnis (#56)
Files changed: - CHANGES.md - VERSION - instructions/page-lifecycle.md - instructions/publish-cycle.md - tools/CONTRACT.md - tools/chemenu/cli.py - tools/chemenu/commands/log_append.py - tools/chemenu/commands/migrate_cmd.py - tools/chemenu/commands/new_page.py - tools/chemenu/commands/page_ops.py - tools/chemenu/corpus_diff.py - tools/chemenu/lint_core.py - tools/chemenu/tests/test_corpus_diff.py - tools/chemenu/tests/test_lint.py - tools/chemenu/tests/test_migrate_cmd.py - tools/chemenu/tests/test_page_ops.py - tools/chemenu/tests/test_type_resolver.py - tools/chemenu/type_resolver.py
This commit is contained in:
@@ -75,6 +75,7 @@ app.command("new")(new_page.new_page_command)
|
||||
app.command("touch")(touch_module.touch_command)
|
||||
app.command("rename")(page_ops.rename_command)
|
||||
app.command("rm")(page_ops.rm_command)
|
||||
app.command("move")(page_ops.move_command)
|
||||
app.command("lint")(lint_module.lint_command)
|
||||
app.command("search")(search_module.search_command)
|
||||
app.command("publish")(git_publish.publish_command)
|
||||
|
||||
@@ -12,7 +12,7 @@ from chemenu.commands._util import fail, rel_path, success, today_iso
|
||||
|
||||
app = typer.Typer(help="Manage wiki/log.md.")
|
||||
|
||||
VALID_OPS = ["ingest", "query", "lint", "create", "update", "delete", "rename"]
|
||||
VALID_OPS = ["ingest", "query", "lint", "create", "update", "delete", "rename", "move"]
|
||||
|
||||
# Matches the "## [YYYY-MM-DD] op | title" heading `format_log_entry` writes,
|
||||
# in file order (oldest first, since entries are appended).
|
||||
|
||||
@@ -378,7 +378,15 @@ def _paths_at(rev: str) -> Optional[list[str]]:
|
||||
|
||||
|
||||
def _shapes_at_revision(rev: str, wanted: set[str]) -> dict[str, corpus_diff.PageShape]:
|
||||
"""Page shapes as of `rev`, keyed by repo-relative path."""
|
||||
"""Page shapes as of `rev`, keyed by page title (the wiki's only identity
|
||||
for a page), not by path - a page that only moved directory between `rev`
|
||||
and now must compare as itself, not as one revision's delete plus the
|
||||
other's add. `PageShape.path` still carries the path, for `moved`.
|
||||
|
||||
Sorted path order, matching `kb_scan.load_kb_pages`: if two paths share a
|
||||
stem (a naming collision `lint` already reports as `duplicate_titles`),
|
||||
the later one wins here too, rather than raising mid-comparison.
|
||||
"""
|
||||
import tempfile
|
||||
|
||||
shapes: dict[str, corpus_diff.PageShape] = {}
|
||||
@@ -388,7 +396,7 @@ def _shapes_at_revision(rev: str, wanted: set[str]) -> dict[str, corpus_diff.Pag
|
||||
return shapes
|
||||
|
||||
with tempfile.TemporaryDirectory() as tmp:
|
||||
for relative in paths:
|
||||
for relative in sorted(paths):
|
||||
if wanted and not any(relative.startswith(prefix) for prefix in wanted):
|
||||
continue
|
||||
text = _git_show(rev, relative)
|
||||
@@ -403,13 +411,14 @@ def _shapes_at_revision(rev: str, wanted: set[str]) -> dict[str, corpus_diff.Pag
|
||||
frontmatter, body = read_page(scratch)
|
||||
except Exception: # noqa: BLE001 - an unparseable historical page is not this tool's error
|
||||
continue
|
||||
shapes[relative] = corpus_diff.PageShape.of(Page(Path(relative), frontmatter, body))
|
||||
shape = corpus_diff.PageShape.of(Page(Path(relative), frontmatter, body))
|
||||
shapes[shape.title] = shape
|
||||
return shapes
|
||||
|
||||
|
||||
def _shapes_now(wanted: set[str]) -> dict[str, corpus_diff.PageShape]:
|
||||
shapes: dict[str, corpus_diff.PageShape] = {}
|
||||
for path in kb_scan.iter_kb_pages(config.KB_DIR):
|
||||
for path in sorted(kb_scan.iter_kb_pages(config.KB_DIR)):
|
||||
relative = path.relative_to(config.ROOT).as_posix()
|
||||
if wanted and not any(relative.startswith(prefix) for prefix in wanted):
|
||||
continue
|
||||
@@ -417,7 +426,11 @@ def _shapes_now(wanted: set[str]) -> dict[str, corpus_diff.PageShape]:
|
||||
frontmatter, body = read_page(path)
|
||||
except Exception: # noqa: BLE001 - lint reports unreadable frontmatter
|
||||
continue
|
||||
shapes[relative] = corpus_diff.PageShape.of(Page(path, frontmatter, body))
|
||||
# The relative path, not `path` itself, so `PageShape.path` is
|
||||
# comparable to the historical side's - both repo-relative - and a
|
||||
# `moved` entry names a path rather than this machine's tmp_path.
|
||||
shape = corpus_diff.PageShape.of(Page(Path(relative), frontmatter, body))
|
||||
shapes[shape.title] = shape
|
||||
return shapes
|
||||
|
||||
|
||||
@@ -439,6 +452,11 @@ def verify_command(
|
||||
must not change: wikilink and citation *counts*, footnote definitions, H1,
|
||||
and structural frontmatter.
|
||||
|
||||
Pages are matched by title, not path, so a page that only moved directory
|
||||
(see `wikitool move`) compares as itself rather than as a
|
||||
removed-and-added pair - its path change is reported separately, as
|
||||
`moved`, and never counted as a finding on its own.
|
||||
|
||||
Not migration-specific - worth running after any bulk rewrite. `lint` cannot
|
||||
answer this: it reads one revision, so a reference that went missing is
|
||||
invisible to it."""
|
||||
@@ -455,6 +473,9 @@ def verify_command(
|
||||
"compared": diff.compared,
|
||||
"added": diff.added,
|
||||
"removed": diff.removed,
|
||||
"moved": [
|
||||
{"page": title, "from": old, "to": new} for title, old, new in diff.moved
|
||||
],
|
||||
"findings": [
|
||||
{"path": f.path, "kind": f.kind, "detail": f.detail} for f in diff.findings
|
||||
],
|
||||
|
||||
@@ -194,46 +194,29 @@ def _apply_template_variables(template: str, variables: Dict[str, Any]) -> str:
|
||||
|
||||
def _page_subdir(subtype: Optional[str], type_path: str) -> Optional[str]:
|
||||
"""Return the subtype-driven subdirectory under a type's `base_dir`, from
|
||||
the type-spec's own `layout:` frontmatter. Returns None for types with no
|
||||
`layout:` (flat directory). Falls back to `<subtype>s` for a subtype the
|
||||
layout doesn't list, matching the previous hand-maintained behavior."""
|
||||
if subtype is None:
|
||||
return None
|
||||
try:
|
||||
layout = resolver.get_layout(type_path)
|
||||
except ValueError:
|
||||
layout = None
|
||||
if layout is None:
|
||||
return None
|
||||
return layout.get(subtype, {}).get("dir", subtype + "s")
|
||||
the type-spec's own `layout:` frontmatter. Thin wrapper over
|
||||
`TypeResolver.subtype_dir` - the one place this computation lives, shared
|
||||
with `move` and `lint`'s misplaced-page finding."""
|
||||
return resolver.subtype_dir(type_path, subtype)
|
||||
|
||||
|
||||
def _target_dir(type_path: str, frontmatter: Dict[str, Any]) -> Path:
|
||||
"""Resolve where an instance of this type is written: `<root>/<base_dir>`,
|
||||
plus a subtype subdirectory when the type declares a `layout:`.
|
||||
|
||||
Thin wrapper over `TypeResolver.compute_target_dir` - the one placement
|
||||
rule, also used by `move` and `lint`'s misplaced-page finding -
|
||||
converting its `ValueError` into the CLI's normal friendly-failure path.
|
||||
|
||||
`base_dir` is resolved against `config.KB_DIR` by default, so tests that
|
||||
point KB_DIR at a temporary fixture wiki can never write into the real
|
||||
`kb/`. A type-spec declaring `root: repo` resolves against `config.ROOT`
|
||||
instead - for artifacts that are agent-directed material rather than
|
||||
knowledge, and so live outside the knowledge layer."""
|
||||
base_dir = resolver.get_base_dir(type_path)
|
||||
if not base_dir:
|
||||
fail(
|
||||
f"Type {type_path} declares no `base_dir:` and cannot be "
|
||||
f"instantiated as a page"
|
||||
)
|
||||
try:
|
||||
root = resolver.get_root(type_path)
|
||||
return resolver.compute_target_dir(type_path, frontmatter)
|
||||
except ValueError as exc:
|
||||
fail(str(exc))
|
||||
target = (config.ROOT if root == "repo" else config.KB_DIR) / base_dir
|
||||
subtype_field = resolver.get_subtype_field(type_path)
|
||||
if subtype_field:
|
||||
subdir = _page_subdir(frontmatter.get(subtype_field), type_path)
|
||||
if subdir:
|
||||
target = target / subdir
|
||||
return target
|
||||
|
||||
|
||||
def _validate_or_fail(frontmatter: Dict[str, Any], type_path: str, source_dir: Path) -> None:
|
||||
|
||||
@@ -28,6 +28,7 @@ import typer
|
||||
from chemenu import config, links
|
||||
from chemenu.commands._util import check_collision, fail, rel_path, success
|
||||
from chemenu.frontmatter_io import write_page
|
||||
from chemenu.lint_core import find_misplaced
|
||||
from chemenu.page import Page
|
||||
from chemenu.kb_scan import load_kb_pages
|
||||
from chemenu.provenance import (
|
||||
@@ -378,3 +379,100 @@ def rm_command(
|
||||
f"Deleted '{page_title}' ({rel_path(target.path)}); de-linked {len(touched)} page(s). "
|
||||
"Run `wikitool index rebuild` and `wikitool sources rebuild-index` next."
|
||||
)
|
||||
|
||||
|
||||
def move_command(
|
||||
page_title: Optional[str] = typer.Option(
|
||||
None, "--page", help="Exact title of the page to move to its computed location"
|
||||
),
|
||||
reconcile: bool = typer.Option(
|
||||
False, "--reconcile", help="Move every page under wiki/ that is not at its computed location"
|
||||
),
|
||||
dry_run: bool = typer.Option(False, "--dry-run", help="List what would move without writing"),
|
||||
):
|
||||
"""Move a page (or every misplaced page) to the directory its type-spec
|
||||
computes for its current frontmatter - `base_dir` + `layout`, the same
|
||||
rule `new` places a page by when it is first created.
|
||||
|
||||
The destination is never chosen by hand: there is no `--to <dir>`. Only
|
||||
the file moves - no body, no frontmatter field, and the page's title (its
|
||||
only identity in the wiki) never changes.
|
||||
"""
|
||||
if bool(page_title) == bool(reconcile):
|
||||
fail('Pass exactly one of --page "<Title>" or --reconcile.')
|
||||
|
||||
pages = load_kb_pages(config.KB_DIR)
|
||||
|
||||
if reconcile:
|
||||
candidates = find_misplaced(pages)
|
||||
if not candidates:
|
||||
success("Nothing to move; every page is already at its computed location.")
|
||||
return
|
||||
|
||||
planned: list[tuple[str, Page, Path, Path]] = []
|
||||
collisions: list[str] = []
|
||||
for title, page, target_dir in candidates:
|
||||
new_path = target_dir / f"{title}.md"
|
||||
if new_path.exists():
|
||||
collisions.append(f"{title} (target {rel_path(new_path)} already exists)")
|
||||
continue
|
||||
planned.append((title, page, target_dir, new_path))
|
||||
|
||||
if dry_run:
|
||||
for title, page, _target_dir, new_path in planned:
|
||||
typer.echo(f"[dry-run] would move {rel_path(page.path)} -> {rel_path(new_path)}")
|
||||
if collisions:
|
||||
typer.echo("")
|
||||
typer.echo("Skipped (target already exists) - resolve with `wikitool rename` first:")
|
||||
for collision in collisions:
|
||||
typer.echo(f" - {collision}")
|
||||
typer.echo(f"[dry-run] would move {len(planned)} page(s). No files written.")
|
||||
return
|
||||
|
||||
moved: list[str] = []
|
||||
failed: list[str] = list(collisions)
|
||||
for title, page, target_dir, new_path in planned:
|
||||
try:
|
||||
target_dir.mkdir(parents=True, exist_ok=True)
|
||||
page.path.rename(new_path)
|
||||
except OSError as exc:
|
||||
failed.append(f"{title} ({exc})")
|
||||
continue
|
||||
moved.append(title)
|
||||
typer.echo(f" moved '{title}' -> {rel_path(new_path)}")
|
||||
|
||||
if failed:
|
||||
fail(
|
||||
f"Moved {len(moved)}/{len(candidates)} page(s) before a failure: {', '.join(failed)}. "
|
||||
"Safe to retry - `move --reconcile` only re-moves what is still misplaced."
|
||||
)
|
||||
success(f"Moved {len(moved)} page(s). Run `wikitool index rebuild` next.")
|
||||
return
|
||||
|
||||
target = pages.get(page_title)
|
||||
if target is None:
|
||||
fail(f"No page titled '{page_title}' found under wiki/.")
|
||||
|
||||
type_path = target.frontmatter.get("type")
|
||||
if not type_path:
|
||||
fail(f"'{page_title}' has no `type:` field, so no placement can be computed for it.")
|
||||
try:
|
||||
target_dir = resolver.compute_target_dir(type_path, target.frontmatter, target.path)
|
||||
except ValueError as exc:
|
||||
fail(str(exc))
|
||||
|
||||
if target_dir.resolve() == target.path.parent.resolve():
|
||||
success(f"'{page_title}' is already at its computed location ({rel_path(target.path)}); nothing to move.")
|
||||
return
|
||||
|
||||
new_path = target_dir / f"{page_title}.md"
|
||||
if new_path.exists():
|
||||
fail(f"Cannot move '{page_title}': {rel_path(new_path)} already exists.")
|
||||
|
||||
if dry_run:
|
||||
typer.echo(f"[dry-run] would move {rel_path(target.path)} -> {rel_path(new_path)}")
|
||||
return
|
||||
|
||||
target_dir.mkdir(parents=True, exist_ok=True)
|
||||
target.path.rename(new_path)
|
||||
success(f"Moved '{page_title}' -> {rel_path(new_path)}. Run `wikitool index rebuild` next.")
|
||||
|
||||
@@ -54,6 +54,7 @@ class PageShape:
|
||||
"""Everything about a page that a content migration must preserve."""
|
||||
|
||||
title: str
|
||||
path: str
|
||||
h1: Optional[str]
|
||||
wikilinks: Counter
|
||||
cite_refs: Counter
|
||||
@@ -77,6 +78,7 @@ class PageShape:
|
||||
fields[subtype_field] = page.frontmatter.get(subtype_field)
|
||||
return cls(
|
||||
title=page.title,
|
||||
path=str(page.path),
|
||||
h1=page.h1_title,
|
||||
wikilinks=kb_scan.count_wikilinks(head),
|
||||
cite_refs=Counter(m.group(1) for m in provenance.CITE_REF_RE.finditer(head)),
|
||||
@@ -109,7 +111,7 @@ def _subtype_field(page: Page) -> Optional[str]:
|
||||
|
||||
@dataclass
|
||||
class PageFinding:
|
||||
path: str
|
||||
path: str # the page's title (`compare()`'s key), kept named `path` for JSON stability
|
||||
kind: str # "h1" | "wikilinks" | "cite-refs" | "cite-defs" | "frontmatter" | "unchanged"
|
||||
detail: str
|
||||
|
||||
@@ -122,6 +124,7 @@ class CorpusDiff:
|
||||
findings: list[PageFinding] = field(default_factory=list)
|
||||
added: list[str] = field(default_factory=list)
|
||||
removed: list[str] = field(default_factory=list)
|
||||
moved: list[tuple[str, str, str]] = field(default_factory=list)
|
||||
compared: int = 0
|
||||
|
||||
@property
|
||||
@@ -143,19 +146,19 @@ def _counter_delta(before: Counter, after: Counter) -> str:
|
||||
return ", ".join(parts)
|
||||
|
||||
|
||||
def compare_page(path: str, before: PageShape, after: PageShape) -> list[PageFinding]:
|
||||
def compare_page(title: str, before: PageShape, after: PageShape) -> list[PageFinding]:
|
||||
findings: list[PageFinding] = []
|
||||
|
||||
if before.h1 != after.h1:
|
||||
findings.append(
|
||||
PageFinding(path, "h1", f"{before.h1!r} -> {after.h1!r} (the title is the page's only identifier)")
|
||||
PageFinding(title, "h1", f"{before.h1!r} -> {after.h1!r} (the title is the page's only identifier)")
|
||||
)
|
||||
|
||||
if before.wikilinks != after.wikilinks:
|
||||
findings.append(PageFinding(path, "wikilinks", _counter_delta(before.wikilinks, after.wikilinks)))
|
||||
findings.append(PageFinding(title, "wikilinks", _counter_delta(before.wikilinks, after.wikilinks)))
|
||||
|
||||
if before.cite_refs != after.cite_refs:
|
||||
findings.append(PageFinding(path, "cite-refs", _counter_delta(before.cite_refs, after.cite_refs)))
|
||||
findings.append(PageFinding(title, "cite-refs", _counter_delta(before.cite_refs, after.cite_refs)))
|
||||
|
||||
if before.cite_defs != after.cite_defs:
|
||||
changed = []
|
||||
@@ -163,7 +166,7 @@ def compare_page(path: str, before: PageShape, after: PageShape) -> list[PageFin
|
||||
was, now = before.cite_defs.get(cite_id), after.cite_defs.get(cite_id)
|
||||
if was != now:
|
||||
changed.append(f"[^{cite_id}] {was!r} -> {now!r}")
|
||||
findings.append(PageFinding(path, "cite-defs", ", ".join(changed)))
|
||||
findings.append(PageFinding(title, "cite-defs", ", ".join(changed)))
|
||||
|
||||
# A generated region that lost or gained a marker is the failure mode the
|
||||
# delimiters were introduced against, and it is silent: a lost opening
|
||||
@@ -180,7 +183,7 @@ def compare_page(path: str, before: PageShape, after: PageShape) -> list[PageFin
|
||||
was, now = before.markers.get(name, 0), after.markers.get(name, 0)
|
||||
if was != now:
|
||||
changed_regions.append(f"{name}: {was} -> {now}")
|
||||
findings.append(PageFinding(path, "markers", ", ".join(changed_regions)))
|
||||
findings.append(PageFinding(title, "markers", ", ".join(changed_regions)))
|
||||
|
||||
changed_fields = []
|
||||
for name in sorted(set(before.fields) | set(after.fields)):
|
||||
@@ -188,7 +191,7 @@ def compare_page(path: str, before: PageShape, after: PageShape) -> list[PageFin
|
||||
if was != now:
|
||||
changed_fields.append(f"{name}: {was!r} -> {now!r}")
|
||||
if changed_fields:
|
||||
findings.append(PageFinding(path, "frontmatter", "; ".join(changed_fields)))
|
||||
findings.append(PageFinding(title, "frontmatter", "; ".join(changed_fields)))
|
||||
|
||||
return findings
|
||||
|
||||
@@ -198,7 +201,12 @@ def compare(
|
||||
after: dict[str, PageShape],
|
||||
expect_body_change: bool = False,
|
||||
) -> CorpusDiff:
|
||||
"""Compare two revisions' page shapes, keyed by repo-relative path.
|
||||
"""Compare two revisions' page shapes, keyed by page title (the wiki's
|
||||
only identity for a page - see AGENTS.md invariant 2), not by path. A page
|
||||
that only changed directory therefore compares as itself rather than as a
|
||||
removed-and-added pair; its path change is reported separately, in
|
||||
`moved`, and is never a finding on its own - a migration is allowed to
|
||||
relocate a page, only not to change what it says.
|
||||
|
||||
`expect_body_change` turns the opposite question on: report a page whose
|
||||
body is byte-identical. A migration unit that reports no such page did
|
||||
@@ -208,12 +216,14 @@ def compare(
|
||||
diff.added = sorted(set(after) - set(before))
|
||||
diff.removed = sorted(set(before) - set(after))
|
||||
|
||||
for path in sorted(set(before) & set(after)):
|
||||
for title in sorted(set(before) & set(after)):
|
||||
diff.compared += 1
|
||||
diff.findings.extend(compare_page(path, before[path], after[path]))
|
||||
if expect_body_change and before[path].body == after[path].body:
|
||||
diff.findings.extend(compare_page(title, before[title], after[title]))
|
||||
if before[title].path != after[title].path:
|
||||
diff.moved.append((title, before[title].path, after[title].path))
|
||||
if expect_body_change and before[title].body == after[title].body:
|
||||
diff.findings.append(
|
||||
PageFinding(path, "unchanged", "body is byte-identical, but this unit claimed to rewrite it")
|
||||
PageFinding(title, "unchanged", "body is byte-identical, but this unit claimed to rewrite it")
|
||||
)
|
||||
|
||||
return diff
|
||||
@@ -223,7 +233,7 @@ def render_report(diff: CorpusDiff, from_rev: str) -> str:
|
||||
lines = [f"# Corpus diff against {from_rev}", ""]
|
||||
lines.append(
|
||||
f"{diff.compared} page(s) compared, {len(diff.added)} added, "
|
||||
f"{len(diff.removed)} removed, {len(diff.findings)} finding(s)."
|
||||
f"{len(diff.removed)} removed, {len(diff.moved)} moved, {len(diff.findings)} finding(s)."
|
||||
)
|
||||
lines.append("")
|
||||
|
||||
@@ -243,4 +253,10 @@ def render_report(diff: CorpusDiff, from_rev: str) -> str:
|
||||
lines += [f"- {path}" for path in paths]
|
||||
lines.append("")
|
||||
|
||||
if diff.moved:
|
||||
lines.append("## Moved pages")
|
||||
lines.append("")
|
||||
lines += [f"- {title}: `{old}` -> `{new}`" for title, old, new in diff.moved]
|
||||
lines.append("")
|
||||
|
||||
return "\n".join(lines)
|
||||
|
||||
@@ -27,6 +27,7 @@ from chemenu.provenance import legacy_source_pages as find_legacy_source_pages
|
||||
from chemenu.provenance import orphan_footnote_defs as find_orphan_footnote_defs
|
||||
from chemenu.provenance import uncovered_raw_files as find_uncovered_raw_files
|
||||
from chemenu.provenance import undefined_footnote_refs as find_undefined_footnote_refs
|
||||
from chemenu.page import Page
|
||||
from chemenu.version import Version
|
||||
from chemenu.kb_scan import (
|
||||
GENERATED_INDEX,
|
||||
@@ -76,6 +77,52 @@ def count_quote_blocks(body: str) -> int:
|
||||
return count
|
||||
|
||||
|
||||
def _display(path: Path) -> str:
|
||||
"""A path for a report line: repo-root-relative if possible, the raw path
|
||||
otherwise (a fixture tree in a test, or any tree `config.ROOT` does not
|
||||
contain)."""
|
||||
try:
|
||||
return str(path.relative_to(config.ROOT))
|
||||
except ValueError:
|
||||
return str(path)
|
||||
|
||||
|
||||
def find_misplaced(pages: dict[str, Page]) -> list[tuple[str, Page, Path]]:
|
||||
"""(title, page, target_dir) for every page whose type resolves and whose
|
||||
current directory differs from the one `TypeResolver.compute_target_dir`
|
||||
would place it under - the same rule `new` places a page by. Shared by
|
||||
`misplaced_pages()` below and `wikitool move --reconcile`, which is the
|
||||
fix for what this finds.
|
||||
|
||||
A page with no `type:` or an unresolvable one is skipped - each is
|
||||
reported separately, as `frontmatter_errors`/`type_resolution_errors`."""
|
||||
found = []
|
||||
for title, page in sorted(pages.items()):
|
||||
type_path = page.frontmatter.get("type")
|
||||
if not type_path:
|
||||
continue
|
||||
try:
|
||||
target_dir = resolver.compute_target_dir(type_path, page.frontmatter, page.path)
|
||||
except ValueError:
|
||||
continue
|
||||
if page.path.parent.resolve() != target_dir.resolve():
|
||||
found.append((title, page, target_dir))
|
||||
return found
|
||||
|
||||
|
||||
def misplaced_pages(pages: dict[str, Page]) -> list[dict]:
|
||||
"""Report form of `find_misplaced`: `{"page", "at", "should_be"}` per
|
||||
finding, for the lint report.
|
||||
|
||||
Advisory rather than a hard error (see `HARD_ERROR_KEYS` below): plenty of
|
||||
existing instances predate `move`, and a hand-placed page is not a
|
||||
broken one - only one `wikitool move --page "<Title>"` would relocate."""
|
||||
return [
|
||||
{"page": title, "at": _display(page.path.parent), "should_be": _display(target_dir)}
|
||||
for title, page, target_dir in find_misplaced(pages)
|
||||
]
|
||||
|
||||
|
||||
def run_lint(kb_dir: Path) -> dict:
|
||||
pages = load_kb_pages(kb_dir)
|
||||
duplicate_titles = find_duplicate_title_paths(kb_dir, config.ROOT)
|
||||
@@ -139,6 +186,8 @@ def run_lint(kb_dir: Path) -> dict:
|
||||
if h1 is not None and h1 != title:
|
||||
title_mismatches.append({"page": title, "h1": h1})
|
||||
|
||||
misplaced = misplaced_pages(pages)
|
||||
|
||||
unmarked_provenance = []
|
||||
for title, page in sorted(pages.items()):
|
||||
if page.kind not in ("entity", "concept"):
|
||||
@@ -318,6 +367,7 @@ def run_lint(kb_dir: Path) -> dict:
|
||||
"dangling_index_entries": dangling_index_entries,
|
||||
"title_mismatches": title_mismatches,
|
||||
"duplicate_titles": duplicate_titles,
|
||||
"misplaced_pages": misplaced,
|
||||
"uncovered_raw_files": find_uncovered_raw_files(config.RAW_DIR, pages),
|
||||
"broken_raw_refs": find_broken_raw_refs(pages),
|
||||
"duplicate_raw_file_owners": find_duplicate_raw_file_owners(pages),
|
||||
@@ -381,6 +431,12 @@ def render_markdown(report: dict) -> str:
|
||||
lines, "Filename / H1 Title Mismatches", report["title_mismatches"],
|
||||
lambda i: f"[[{i['page']}]] H1 is '{i['h1']}'",
|
||||
)
|
||||
_section(
|
||||
lines, "Misplaced Pages (not under their type-spec's computed directory)",
|
||||
report.get("misplaced_pages", []),
|
||||
lambda i: f"[[{i['page']}]] is at `{i['at']}`, should be under `{i['should_be']}` "
|
||||
f"- `wikitool move --page \"{i['page']}\"`",
|
||||
)
|
||||
_section(
|
||||
lines, "Uncovered Raw Files (no source page)", report["uncovered_raw_files"],
|
||||
lambda i: f"`{i}`",
|
||||
@@ -529,6 +585,11 @@ def default_report_path(report: dict) -> Path:
|
||||
# is no version at which the redundancy becomes an error, only a sweep someone
|
||||
# does or does not get to.
|
||||
#
|
||||
# `misplaced_pages` is the same shape again: it arrived long after most
|
||||
# instances' corpora were hand-placed, a hand-placed page is not a broken one,
|
||||
# and there is no version at which "not under the computed directory" becomes
|
||||
# wrong - only `wikitool move` someone does or does not get to run.
|
||||
#
|
||||
# `malformed_edges` and `unbalanced_markers` are hard from the start: neither
|
||||
# describes an unconverted page, only a broken one.
|
||||
#
|
||||
|
||||
@@ -147,3 +147,33 @@ def test_report_renders_a_clean_run_explicitly():
|
||||
report = corpus_diff.render_report(corpus_diff.compare(*shapes(BODY, BODY)), "HEAD")
|
||||
assert "1 page(s) compared" in report
|
||||
assert "No invariant changed" in report
|
||||
|
||||
|
||||
def test_a_page_that_only_moved_directory_compares_as_itself():
|
||||
"""A move is not a rewrite: same title, same body, different path. It must
|
||||
show up as `moved`, never as a removed-and-added pair, and never as a
|
||||
finding on its own."""
|
||||
before = Page(Path("kb/entities/systems/Aurora.md"), page(BODY).frontmatter, BODY)
|
||||
after = Page(Path("kb/entities/technologies/Aurora.md"), page(BODY).frontmatter, BODY)
|
||||
diff = corpus_diff.compare(
|
||||
{"Aurora": corpus_diff.PageShape.of(before)},
|
||||
{"Aurora": corpus_diff.PageShape.of(after)},
|
||||
)
|
||||
assert diff.added == []
|
||||
assert diff.removed == []
|
||||
assert diff.compared == 1
|
||||
assert diff.findings == []
|
||||
assert diff.moved == [("Aurora", "kb/entities/systems/Aurora.md", "kb/entities/technologies/Aurora.md")]
|
||||
|
||||
|
||||
def test_moved_pages_render_in_the_report_without_being_a_finding():
|
||||
before = Page(Path("kb/entities/systems/Aurora.md"), page(BODY).frontmatter, BODY)
|
||||
after = Page(Path("kb/entities/technologies/Aurora.md"), page(BODY).frontmatter, BODY)
|
||||
diff = corpus_diff.compare(
|
||||
{"Aurora": corpus_diff.PageShape.of(before)},
|
||||
{"Aurora": corpus_diff.PageShape.of(after)},
|
||||
)
|
||||
report = corpus_diff.render_report(diff, "HEAD")
|
||||
assert "1 moved" in report
|
||||
assert "## Moved pages" in report
|
||||
assert "No invariant changed" in report
|
||||
|
||||
@@ -184,6 +184,35 @@ def test_clean_wiki_has_no_hard_errors(kb_dir):
|
||||
assert report["duplicate_titles"] == []
|
||||
|
||||
|
||||
def test_lint_detects_a_misplaced_page(kb_dir):
|
||||
write_page(
|
||||
kb_dir / "entities/systems/misplaced-tool.md",
|
||||
{"type": "types/entity.md", "entity_type": "tool", "tags": [], "created": "2026-07-25",
|
||||
"modified": "2026-07-25", "related": [], "sources": [], "confidence": 0.8},
|
||||
"\n# misplaced-tool\n",
|
||||
)
|
||||
report = run_lint(kb_dir)
|
||||
entry = next(i for i in report["misplaced_pages"] if i["page"] == "misplaced-tool")
|
||||
assert entry["at"].endswith("entities/systems")
|
||||
assert entry["should_be"].endswith("entities/tools")
|
||||
# Advisory, not a hard error - a corpus whose only finding is this one
|
||||
# must stay green (see test_a_lint_run_with_only_a_misplaced_page_is_green).
|
||||
assert "misplaced_pages" not in HARD_ERROR_KEYS
|
||||
|
||||
|
||||
def test_a_report_with_only_a_misplaced_page_stays_green():
|
||||
report = {key: [] for key in HARD_ERROR_KEYS}
|
||||
report["misplaced_pages"] = [{"page": "x", "at": "a", "should_be": "b"}]
|
||||
assert not has_hard_errors(report)
|
||||
|
||||
|
||||
def test_lint_is_silent_about_correctly_placed_pages(kb_dir):
|
||||
"""The fixture wiki's own pages (aurora under entities/systems/,
|
||||
gdeploy under entities/tools/, ...) are all at their computed
|
||||
location - the check must not fire on them."""
|
||||
assert run_lint(kb_dir)["misplaced_pages"] == []
|
||||
|
||||
|
||||
def test_lint_flags_legacy_citation_marker_as_hard_error(kb_dir):
|
||||
write_page(
|
||||
kb_dir / "concepts/Modbus.md",
|
||||
|
||||
@@ -280,6 +280,63 @@ def test_verify_does_not_mistake_routing_files_for_removed_pages(git_instance, c
|
||||
assert result["compared"] == 1
|
||||
|
||||
|
||||
def test_verify_treats_a_moved_page_as_the_same_page(git_instance, capsys):
|
||||
"""Regression for Gitea #56: before the stem-keying fix, a pure `page
|
||||
move` reported `1 removed, 1 added, 0 compared` and the one mechanical
|
||||
check a migration has never actually ran."""
|
||||
moved_dir = git_instance / "kb" / "entities" / "moved"
|
||||
moved_dir.mkdir()
|
||||
src = git_instance / "kb" / "entities" / "Aurora.md"
|
||||
dst = moved_dir / "Aurora.md"
|
||||
src.rename(dst)
|
||||
migrate_cmd.verify_command(
|
||||
from_rev="HEAD", path=None, expect_body_change=False, json_out=True, fail_on_error=True
|
||||
)
|
||||
result = json.loads(capsys.readouterr().out)
|
||||
assert result["compared"] == 1
|
||||
assert result["added"] == []
|
||||
assert result["removed"] == []
|
||||
assert result["findings"] == []
|
||||
assert result["moved"] == [
|
||||
{"page": "Aurora", "from": "kb/entities/Aurora.md", "to": "kb/entities/moved/Aurora.md"}
|
||||
]
|
||||
|
||||
|
||||
def test_verify_confirms_three_moved_pages_compare_as_themselves(tmp_path, monkeypatch, capsys):
|
||||
"""Acceptance regression for #56: move 3 pages, `verify` runs with
|
||||
`compared == 3, added == 0, removed == 0`."""
|
||||
kb = tmp_path / "kb" / "entities"
|
||||
kb.mkdir(parents=True)
|
||||
(tmp_path / "kb" / "entities" / "COLLECTION.md").write_text("# c\n", encoding="utf-8")
|
||||
for name in ("Alpha", "Beta", "Gamma"):
|
||||
(kb / f"{name}.md").write_text(PAGE.replace("Aurora", name), encoding="utf-8")
|
||||
|
||||
subprocess.run(["git", "init", "-q", "-b", "main"], cwd=tmp_path, check=True)
|
||||
subprocess.run(["git", "config", "user.name", "T"], cwd=tmp_path, check=True)
|
||||
subprocess.run(["git", "config", "user.email", "t@e.invalid"], cwd=tmp_path, check=True)
|
||||
subprocess.run(["git", "add", "-A"], cwd=tmp_path, check=True)
|
||||
subprocess.run(
|
||||
["git", "commit", "-q", "-m", "seed"], cwd=tmp_path, check=True, capture_output=True
|
||||
)
|
||||
|
||||
monkeypatch.setattr(config, "ROOT", tmp_path)
|
||||
monkeypatch.setattr(config, "KB_DIR", tmp_path / "kb")
|
||||
|
||||
moved_dir = kb / "moved"
|
||||
moved_dir.mkdir()
|
||||
for name in ("Alpha", "Beta", "Gamma"):
|
||||
(kb / f"{name}.md").rename(moved_dir / f"{name}.md")
|
||||
|
||||
migrate_cmd.verify_command(
|
||||
from_rev="HEAD", path=None, expect_body_change=False, json_out=True, fail_on_error=True
|
||||
)
|
||||
result = json.loads(capsys.readouterr().out)
|
||||
assert result["compared"] == 3
|
||||
assert result["added"] == []
|
||||
assert result["removed"] == []
|
||||
assert {m["page"] for m in result["moved"]} == {"Alpha", "Beta", "Gamma"}
|
||||
|
||||
|
||||
def test_verify_reports_an_unknown_revision(git_instance):
|
||||
with pytest.raises(typer.Exit):
|
||||
migrate_cmd.verify_command(
|
||||
|
||||
@@ -1,3 +1,5 @@
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
import typer
|
||||
|
||||
@@ -286,3 +288,93 @@ def test_inbound_pages_sees_frontmatter_only_references(patched_wiki):
|
||||
"\n# gdeploy\n\n## Description\n\nNo body link at all.\n",
|
||||
)
|
||||
assert "gdeploy" in page_ops.inbound_pages(load_kb_pages(patched_wiki), "Modbus")
|
||||
|
||||
|
||||
# --- move --------------------------------------------------------------
|
||||
|
||||
|
||||
def _write_misplaced(kb: Path, relative: str, title: str, entity_type: str) -> None:
|
||||
write_page(
|
||||
kb / relative / f"{title}.md",
|
||||
{
|
||||
"type": "types/entity.md", "entity_type": entity_type, "tags": [],
|
||||
"created": "2026-07-25", "modified": "2026-07-25",
|
||||
"related": [], "sources": [], "confidence": 0.8,
|
||||
},
|
||||
f"\n# {title}\n",
|
||||
)
|
||||
|
||||
|
||||
def test_move_relocates_a_page_to_its_computed_directory(patched_wiki):
|
||||
_write_misplaced(patched_wiki, "entities/systems", "misplaced-tool", "tool")
|
||||
page_ops.move_command(page_title="misplaced-tool", reconcile=False, dry_run=False)
|
||||
assert not (patched_wiki / "entities/systems/misplaced-tool.md").exists()
|
||||
assert (patched_wiki / "entities/tools/misplaced-tool.md").exists()
|
||||
frontmatter, body = read_page(patched_wiki / "entities/tools/misplaced-tool.md")
|
||||
assert frontmatter["entity_type"] == "tool"
|
||||
assert body.strip() == "# misplaced-tool"
|
||||
|
||||
|
||||
def test_move_dry_run_writes_nothing(patched_wiki):
|
||||
_write_misplaced(patched_wiki, "entities/systems", "misplaced-tool", "tool")
|
||||
page_ops.move_command(page_title="misplaced-tool", reconcile=False, dry_run=True)
|
||||
assert (patched_wiki / "entities/systems/misplaced-tool.md").exists()
|
||||
assert not (patched_wiki / "entities/tools/misplaced-tool.md").exists()
|
||||
|
||||
|
||||
def test_move_is_a_noop_when_already_at_its_computed_location(patched_wiki, capsys):
|
||||
page_ops.move_command(page_title="aurora", reconcile=False, dry_run=False)
|
||||
assert "already at its computed location" in capsys.readouterr().out
|
||||
assert (patched_wiki / "entities/systems/aurora.md").exists()
|
||||
|
||||
|
||||
def test_move_rejects_missing_page(patched_wiki):
|
||||
with pytest.raises(typer.Exit):
|
||||
page_ops.move_command(page_title="No Such Page", reconcile=False, dry_run=False)
|
||||
|
||||
|
||||
def test_move_requires_exactly_one_of_page_or_reconcile(patched_wiki):
|
||||
with pytest.raises(typer.Exit):
|
||||
page_ops.move_command(page_title=None, reconcile=False, dry_run=False)
|
||||
with pytest.raises(typer.Exit):
|
||||
page_ops.move_command(page_title="aurora", reconcile=True, dry_run=False)
|
||||
|
||||
|
||||
def test_move_refuses_when_the_destination_already_exists(patched_wiki):
|
||||
"""A pre-existing duplicate-title situation: two files share the stem
|
||||
`dup`, so `load_kb_pages` (see its own docstring) surfaces only the one
|
||||
that sorts last by path - `entities/zzz-wrong/dup.md` - as the tracked
|
||||
page `dup`. Its computed destination, `entities/tools/dup.md`, is
|
||||
occupied by the other, untracked half of the collision. `move` must
|
||||
refuse rather than silently overwrite it."""
|
||||
_write_misplaced(patched_wiki, "entities/tools", "dup", "tool")
|
||||
(patched_wiki / "entities/zzz-wrong").mkdir()
|
||||
_write_misplaced(patched_wiki, "entities/zzz-wrong", "dup", "tool")
|
||||
with pytest.raises(typer.Exit):
|
||||
page_ops.move_command(page_title="dup", reconcile=False, dry_run=False)
|
||||
assert (patched_wiki / "entities/tools/dup.md").exists()
|
||||
assert (patched_wiki / "entities/zzz-wrong/dup.md").exists()
|
||||
|
||||
|
||||
def test_move_reconcile_moves_every_misplaced_page(patched_wiki, capsys):
|
||||
_write_misplaced(patched_wiki, "entities/systems", "misplaced-tool", "tool")
|
||||
_write_misplaced(patched_wiki, "entities/systems", "misplaced-person", "person")
|
||||
page_ops.move_command(page_title=None, reconcile=True, dry_run=False)
|
||||
|
||||
assert (patched_wiki / "entities/tools/misplaced-tool.md").exists()
|
||||
assert (patched_wiki / "entities/people/misplaced-person.md").exists()
|
||||
assert not (patched_wiki / "entities/systems/misplaced-tool.md").exists()
|
||||
assert not (patched_wiki / "entities/systems/misplaced-person.md").exists()
|
||||
# Already-correct pages are left alone.
|
||||
assert (patched_wiki / "entities/systems/aurora.md").exists()
|
||||
|
||||
capsys.readouterr()
|
||||
page_ops.move_command(page_title=None, reconcile=True, dry_run=False)
|
||||
assert "Nothing to move" in capsys.readouterr().out
|
||||
|
||||
|
||||
def test_move_reconcile_dry_run_writes_nothing(patched_wiki):
|
||||
_write_misplaced(patched_wiki, "entities/systems", "misplaced-tool", "tool")
|
||||
page_ops.move_command(page_title=None, reconcile=True, dry_run=True)
|
||||
assert (patched_wiki / "entities/systems/misplaced-tool.md").exists()
|
||||
assert not (patched_wiki / "entities/tools/misplaced-tool.md").exists()
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
import pytest
|
||||
|
||||
from chemenu import config
|
||||
from chemenu.type_resolver import resolver
|
||||
|
||||
|
||||
@@ -191,3 +192,43 @@ def test_validate_frontmatter_reports_wrong_field_type():
|
||||
},
|
||||
"types/entity.md",
|
||||
)
|
||||
|
||||
|
||||
# --- compute_target_dir / subtype_dir: the one placement rule --------------
|
||||
#
|
||||
# Shared by `new` (writes here), `move` and `lint`'s misplaced-page
|
||||
# finding (both check a page already is here) - see AGENTS.md invariant 8.
|
||||
|
||||
|
||||
def test_subtype_dir_reads_layout_from_type_spec():
|
||||
assert resolver.subtype_dir("types/entity.md", "tool") == "tools"
|
||||
assert resolver.subtype_dir("types/entity.md", "technology") == "technologies"
|
||||
|
||||
|
||||
def test_subtype_dir_falls_back_for_unmapped_subtype():
|
||||
assert resolver.subtype_dir("types/entity.md", "gadget") == "gadgets"
|
||||
|
||||
|
||||
def test_subtype_dir_is_none_without_layout_or_subtype():
|
||||
assert resolver.subtype_dir("types/concept.md", "workflow") is None
|
||||
assert resolver.subtype_dir("types/entity.md", None) is None
|
||||
|
||||
|
||||
def test_compute_target_dir_applies_layout_subdirectory():
|
||||
target = resolver.compute_target_dir("types/entity.md", {"entity_type": "tool"})
|
||||
assert target == config.KB_DIR / "entities" / "tools"
|
||||
|
||||
|
||||
def test_compute_target_dir_is_flat_for_a_type_without_layout():
|
||||
target = resolver.compute_target_dir("types/concept.md", {"concept_type": "workflow"})
|
||||
assert target == config.KB_DIR / "concepts"
|
||||
|
||||
|
||||
def test_compute_target_dir_resolves_against_repo_root_for_root_repo_types():
|
||||
target = resolver.compute_target_dir("types/instruction.md", {})
|
||||
assert target == config.ROOT / "instructions"
|
||||
|
||||
|
||||
def test_compute_target_dir_rejects_a_type_with_no_base_dir():
|
||||
with pytest.raises(ValueError, match="base_dir"):
|
||||
resolver.compute_target_dir("types/type-spec.md", {})
|
||||
|
||||
@@ -359,6 +359,54 @@ class TypeResolver:
|
||||
)
|
||||
return root
|
||||
|
||||
def subtype_dir(self, type_path: str, subtype: Optional[str], source_file: Path = None) -> Optional[str]:
|
||||
"""The subtype-driven subdirectory `layout:` assigns to `subtype`, or
|
||||
None for a type with no `layout:` (flat directory) or a `subtype` of
|
||||
None. A subtype absent from the layout still gets a directory, by
|
||||
pluralizing its own name (`gadget` -> `gadgets`) - the fallback `new`
|
||||
used before `layout:` existed, kept so an unlisted subtype value
|
||||
doesn't refuse placement outright.
|
||||
|
||||
Raises:
|
||||
ValueError: If the type path cannot be resolved.
|
||||
"""
|
||||
if subtype is None:
|
||||
return None
|
||||
try:
|
||||
layout = self.get_layout(type_path, source_file)
|
||||
except ValueError:
|
||||
layout = None
|
||||
if layout is None:
|
||||
return None
|
||||
return layout.get(subtype, {}).get('dir', subtype + 's')
|
||||
|
||||
def compute_target_dir(self, type_path: str, frontmatter: Dict[str, Any], source_file: Path = None) -> Path:
|
||||
"""Where an instance of this type belongs on disk: `<root>/<base_dir>`,
|
||||
plus a subtype subdirectory when the type declares `layout:`.
|
||||
|
||||
The single placement rule behind `new` (which writes here), `move`
|
||||
and `lint`'s misplaced-page finding (which check a page already is
|
||||
here) - one rule, computed once, per AGENTS.md invariant 8.
|
||||
|
||||
Raises:
|
||||
ValueError: The type declares no `base_dir:`, or its type path
|
||||
cannot be resolved (propagated from the underlying `get_*`
|
||||
calls).
|
||||
"""
|
||||
base_dir = self.get_base_dir(type_path, source_file)
|
||||
if not base_dir:
|
||||
raise ValueError(
|
||||
f"Type {type_path} declares no `base_dir:` and cannot be instantiated as a page"
|
||||
)
|
||||
root = self.get_root(type_path, source_file)
|
||||
target = (config.ROOT if root == 'repo' else config.KB_DIR) / base_dir
|
||||
subtype_field = self.get_subtype_field(type_path, source_file)
|
||||
if subtype_field:
|
||||
subdir = self.subtype_dir(type_path, frontmatter.get(subtype_field), source_file)
|
||||
if subdir:
|
||||
target = target / subdir
|
||||
return target
|
||||
|
||||
def get_title_prefix(self, type_path: str, source_file: Path = None) -> str:
|
||||
"""Return a type-spec's `title_prefix:` frontmatter (e.g. 'Source - '
|
||||
for source pages), or an empty string if it declares none - always a
|
||||
|
||||
Reference in New Issue
Block a user