diff --git a/src/basic_memory/indexing/wiki_projector.py b/src/basic_memory/indexing/wiki_projector.py index 8bde91501..5698fca41 100644 --- a/src/basic_memory/indexing/wiki_projector.py +++ b/src/basic_memory/indexing/wiki_projector.py @@ -2,6 +2,7 @@ from __future__ import annotations +from collections.abc import Iterable from dataclasses import dataclass from datetime import datetime, timezone from enum import StrEnum @@ -205,6 +206,24 @@ class WikiProjectionConflict: reason: str +@dataclass(frozen=True, slots=True) +class WikiScopeCollision: + """Folders whose names differ only by letter case or Unicode normalization. + + Their index.md and log.md would be one file on a case-insensitive filesystem, + so none of the spellings is projected until the user renames one of them. + """ + + scopes: tuple[str, ...] + + @property + def message(self) -> str: + return ( + "Wiki skipped folders whose names differ only by case or Unicode " + "normalization; rename one so each has its own index: " + ", ".join(self.scopes) + ) + + @dataclass(frozen=True, slots=True) class WikiProjectionResult: """Portable outcome recorded by local and Cloud run ledgers.""" @@ -217,6 +236,7 @@ class WikiProjectionResult: conflicts: tuple[WikiProjectionConflict, ...] warnings: tuple[str, ...] pending_materialization: tuple[int, ...] + scope_collisions: tuple[WikiScopeCollision, ...] = () @property def state(self) -> WikiProjectionState: @@ -397,57 +417,30 @@ def _plan_wiki_projection( "Wiki source change permalink collides with a generated document identity: " f"{change.permalink}" ) - scope_by_portable_path: dict[str, str] = {} - for scope in scopes: - portable_scope = _portable_path_key(scope) - if existing_note := note_by_path.get(portable_scope): + for scope in (*scopes, *retained_scope_ancestors): + if existing_note := note_by_path.get(_portable_path_key(scope)): raise ValueError( "Wiki projection scope collides with an existing source note path: " f"{scope}, {existing_note.path}" ) - if existing_scope := scope_by_portable_path.get(portable_scope): - raise ValueError( - "Wiki projection scopes must be unique when compared as portable paths: " - f"{existing_scope}, {scope}" - ) - scope_by_portable_path[portable_scope] = scope + note_scopes = affected_wiki_scopes(*(note.path for note in notes)) + scope_collisions = _scope_collisions((*note_scopes, *scopes, *retained_scope_ancestors)) + colliding_scopes = {scope for collision in scope_collisions for scope in collision.scopes} + # Trigger: two folders differ only by case or Unicode normalization + # (`marketing/Teams` and `marketing/teams`). + # Why: their generated index.md and log.md would be one file on a + # case-insensitive sync target, so neither spelling can be projected safely. + # Raising here failed every later run and froze the whole project's Wiki (#1787). + # Outcome: every spelling is left out of this plan and reported on the result; + # all other scopes still project and the watermark advances. + scopes = tuple(scope for scope in scopes if scope not in colliding_scopes) + projected_scopes = tuple( + sorted({*note_scopes, *scopes, *retained_scope_ancestors} - colliding_scopes) + ) existing_by_path = { _portable_path_key(document.path): document for document in snapshot.reserved_documents } - projected_scope_by_portable_path: dict[str, str] = {} - note_scopes = affected_wiki_scopes(*(note.path for note in notes)) - for projected_scope in (*note_scopes, *scopes): - portable_scope = _portable_path_key(projected_scope) - if ( - existing_scope := projected_scope_by_portable_path.get(portable_scope) - ) is not None and existing_scope != projected_scope: - raise ValueError( - "Wiki projected scopes must be unique when compared as portable paths: " - f"{existing_scope}, {projected_scope}" - ) - projected_scope_by_portable_path[portable_scope] = projected_scope - for document in snapshot.reserved_documents: - if ( - not document.projector_owned - or PurePosixPath(document.path).name.casefold() != "index.md" - ): - continue - for projected_scope in affected_wiki_scopes(document.path): - portable_scope = _portable_path_key(projected_scope) - if existing_note := note_by_path.get(portable_scope): - raise ValueError( - "Wiki projection scope collides with an existing source note path: " - f"{projected_scope}, {existing_note.path}" - ) - if ( - existing_scope := projected_scope_by_portable_path.get(portable_scope) - ) is not None and existing_scope != projected_scope: - raise ValueError( - "Wiki projected scopes must be unique when compared as portable paths: " - f"{existing_scope}, {projected_scope}" - ) - projected_scope_by_portable_path[portable_scope] = projected_scope - projected_scopes = tuple(sorted(projected_scope_by_portable_path.values())) + collision_warnings = tuple(collision.message for collision in scope_collisions) rendered: dict[str, bytes] = {} for scope in scopes: rendered[_reserved_path(scope, "index.md")] = _render_index( @@ -488,8 +481,9 @@ def _plan_wiki_projection( updated=0, unchanged=0, conflicts=conflicts, - warnings=(), + warnings=collision_warnings, pending_materialization=(), + scope_collisions=scope_collisions, ), ) @@ -533,8 +527,9 @@ def _plan_wiki_projection( updated=updated, unchanged=len(unchanged_paths), conflicts=(), - warnings=(), + warnings=collision_warnings, pending_materialization=(), + scope_collisions=scope_collisions, ), ) @@ -575,6 +570,18 @@ def _projection_scopes( return tuple(sorted(scopes & current_scopes)) +def _scope_collisions(scopes: Iterable[str]) -> tuple[WikiScopeCollision, ...]: + """Group distinct scope spellings that share one portable path.""" + spellings_by_portable_path: dict[str, set[str]] = {} + for scope in scopes: + spellings_by_portable_path.setdefault(_portable_path_key(scope), set()).add(scope) + return tuple( + WikiScopeCollision(scopes=tuple(sorted(spellings))) + for _portable_path, spellings in sorted(spellings_by_portable_path.items()) + if len(spellings) > 1 + ) + + def _without_projection_metadata(content: bytes) -> bytes: frontmatter, separator, body = content.partition(b"\n---\n") normalized_frontmatter = b"\n".join( @@ -616,7 +623,8 @@ def _render_index( child_scope = _direct_child_projected_scope(scope, projected_scope) if child_scope is not None: child_scope_set.add(child_scope) - child_scopes = sorted(child_scope_set) + # A folder left out of the plan has no index to link to. + child_scopes = sorted(child_scope_set.intersection(projected_scopes)) title = snapshot.project_name if not scope else _display_name(PurePosixPath(scope).name) body: list[str] = [f"# {_escape_generated_markdown_text(title)}", ""] if child_scopes: diff --git a/tests/indexing/test_wiki_projector.py b/tests/indexing/test_wiki_projector.py index 7f7021bd4..e722311b9 100644 --- a/tests/indexing/test_wiki_projector.py +++ b/tests/indexing/test_wiki_projector.py @@ -14,11 +14,13 @@ WikiChangeOperation, WikiProjectionReason, WikiProjectionRequest, + WikiProjectionPlan, WikiProjectionResult, WikiProjectionSnapshot, WikiProjectionState, WIKI_PROJECTOR_VERSION, WikiReservedDocument, + WikiScopeCollision, WikiSourceChange, WikiSourceNote, WIKI_LOG_ENTRY_LIMIT, @@ -106,6 +108,19 @@ def _reserved(path: str, content: bytes, *, owned: bool = True) -> WikiReservedD ) +def _assert_scope_collision_skipped(plan: WikiProjectionPlan, spellings: tuple[str, ...]) -> None: + """A portable-path collision is reported on the result and never projected.""" + assert plan.result.scope_collisions == (WikiScopeCollision(scopes=spellings),) + assert plan.result.warnings == (WikiScopeCollision(scopes=spellings).message,) + assert plan.result.state is WikiProjectionState.current + assert plan.result.output_watermark == plan.request.through_partition_position + written = {write.path: write.content.decode() for write in plan.writes} + assert "index.md" in written + for spelling in spellings: + assert not any(path.startswith(f"{spelling}/") for path in written) + assert not any(f"[[{spelling}/index|" in content for content in written.values()) + + def test_affected_scopes_include_root_and_move_ancestors() -> None: assert affected_wiki_scopes("guides/old/setup.md", "reference/new/setup.md") == ( "", @@ -312,7 +327,7 @@ def test_snapshot_rejects_case_folded_duplicate_reserved_paths() -> None: ("first_scope", "second_scope"), (("Foo", "foo"), ("caf\u00e9", "cafe\u0301")), ) -def test_projection_rejects_nonportable_duplicate_scopes( +def test_projection_skips_nonportable_duplicate_scopes( first_scope: str, second_scope: str, ) -> None: @@ -336,11 +351,12 @@ def test_projection_rejects_nonportable_duplicate_scopes( ), ) - with pytest.raises(ValueError, match="unique when compared as portable paths"): - plan_wiki_projection( - _request(reason=WikiProjectionReason.manual_rebuild, scopes=()), - snapshot, - ) + plan = plan_wiki_projection( + _request(reason=WikiProjectionReason.manual_rebuild, scopes=()), + snapshot, + ) + + _assert_scope_collision_skipped(plan, tuple(sorted((first_scope, second_scope)))) def test_snapshot_rejects_unicode_normalized_duplicate_reserved_paths() -> None: @@ -530,7 +546,7 @@ def test_incremental_projection_preserves_existing_empty_folder_links() -> None: assert "[[guides/index|Guides]]" in rendered["index.md"] -def test_incremental_projection_rejects_case_folded_existing_empty_scope() -> None: +def test_incremental_projection_skips_case_folded_existing_empty_scope() -> None: note = WikiSourceNote( path="foo/note.md", permalink="foo/note", @@ -559,11 +575,12 @@ def test_incremental_projection_rejects_case_folded_existing_empty_scope() -> No reserved_documents=(_reserved("Foo/index.md", b"# Foo\n"),), ) - with pytest.raises(ValueError, match="projected scopes must be unique"): - plan_wiki_projection(_request(position=1, scopes=()), snapshot) + plan = plan_wiki_projection(_request(position=1, scopes=()), snapshot) + + _assert_scope_collision_skipped(plan, ("Foo", "foo")) -def test_incremental_projection_validates_existing_scope_against_unchanged_notes() -> None: +def test_incremental_projection_skips_existing_scope_colliding_with_unchanged_notes() -> None: historical_note = WikiSourceNote( path="foo/historical.md", permalink="foo/historical", @@ -599,8 +616,9 @@ def test_incremental_projection_validates_existing_scope_against_unchanged_notes reserved_documents=(_reserved("Foo/index.md", b"# Foo\n"),), ) - with pytest.raises(ValueError, match="projected scopes must be unique"): - plan_wiki_projection(_request(position=1, scopes=()), snapshot) + plan = plan_wiki_projection(_request(position=1, scopes=()), snapshot) + + _assert_scope_collision_skipped(plan, ("Foo", "foo")) def test_incremental_projection_rejects_existing_scope_at_source_note_path() -> None: @@ -636,7 +654,7 @@ def test_incremental_projection_rejects_existing_scope_at_source_note_path() -> plan_wiki_projection(_request(position=1, scopes=()), snapshot) -def test_incremental_projection_validates_retained_scope_ancestors() -> None: +def test_incremental_projection_skips_retained_scope_ancestor_collisions() -> None: historical_note = WikiSourceNote( path="foo/historical.md", permalink="foo/historical", @@ -672,8 +690,9 @@ def test_incremental_projection_validates_retained_scope_ancestors() -> None: reserved_documents=(_reserved("Foo/deep/index.md", b"# Deep\n"),), ) - with pytest.raises(ValueError, match="projected scopes must be unique"): - plan_wiki_projection(_request(position=1, scopes=()), snapshot) + plan = plan_wiki_projection(_request(position=1, scopes=()), snapshot) + + _assert_scope_collision_skipped(plan, ("Foo", "foo")) def test_incremental_projection_materializes_missing_retained_scope_ancestors() -> None: @@ -1583,3 +1602,89 @@ def test_projection_preserves_literal_entity_looking_titles( rendered = {write.path: write.content.decode() for write in plan.writes} assert f"[[entity-title|{escaped_title}]]" in rendered["index.md"] + + +def _marketing_note(folder: str, name: str) -> WikiSourceNote: + return WikiSourceNote( + path=f"marketing/{folder}/{name}.md", + permalink=f"marketing/{folder.lower()}-{name}", + title=name.title(), + note_type="Note", + checksum=f"{folder}-{name}-checksum", + ) + + +def test_case_only_folder_collision_does_not_fail_the_rebuild() -> None: + """`marketing/Teams` and `marketing/teams` must not freeze the project's Wiki (#1787).""" + base = _snapshot() + snapshot = replace( + base, + notes=( + *base.notes, + _marketing_note("Teams", "roster"), + _marketing_note("teams", "offsite"), + WikiSourceNote( + path="marketing/plan.md", + permalink="marketing/plan", + title="Plan", + note_type="Note", + checksum="plan-checksum", + ), + ), + ) + + plan = plan_wiki_projection( + _request(reason=WikiProjectionReason.manual_rebuild, scopes=()), + snapshot, + ) + + _assert_scope_collision_skipped(plan, ("marketing/Teams", "marketing/teams")) + written = {write.path: write.content.decode() for write in plan.writes} + # Every other scope still projects, including the colliding folders' parent. + assert set(written) == { + "index.md", + "log.md", + "guides/index.md", + "guides/log.md", + "guides/deep/index.md", + "guides/deep/log.md", + "marketing/index.md", + "marketing/log.md", + } + assert "[[marketing/plan|Plan]]" in written["marketing/index.md"] + assert "[[marketing/index|Marketing]]" in written["index.md"] + + +def test_case_only_folder_collision_does_not_fail_an_incremental_run() -> None: + """An accepted edit inside one colliding folder still projects its other ancestors.""" + roster = _marketing_note("Teams", "roster") + offsite = _marketing_note("teams", "offsite") + change = WikiSourceChange( + partition_position=1, + operation=WikiChangeOperation.updated, + path=offsite.path, + permalink=offsite.permalink, + title=offsite.title, + accepted_at=ACCEPTED_AT, + materialized=True, + source="web", + ) + snapshot = WikiProjectionSnapshot( + project_id="project-88", + project_name="Project 88", + source_partition_position=1, + current_output_watermark=0, + source_accepted_at=ACCEPTED_AT, + notes=(roster, offsite), + changes=(change,), + ) + + plan = plan_wiki_projection(_request(position=1, scopes=()), snapshot) + + _assert_scope_collision_skipped(plan, ("marketing/Teams", "marketing/teams")) + assert {write.path for write in plan.writes} == { + "index.md", + "log.md", + "marketing/index.md", + "marketing/log.md", + }