From 1bec33487b34f15017e73e44a0cc455a8065bf26 Mon Sep 17 00:00:00 2001 From: Vasily Ilin Date: Fri, 25 Sep 2026 20:20:26 -0700 Subject: [PATCH 1/2] fix: preserve title-first YAML cards during automatic rebases --- python/lean_pool/rebase.py | 57 +++++++++++++++++++++++++----- python/tests/test_rebase.py | 70 +++++++++++++++++++++++++++++++++++++ 2 files changed, 118 insertions(+), 9 deletions(-) diff --git a/python/lean_pool/rebase.py b/python/lean_pool/rebase.py index 602bbadd18..51165f8fb4 100644 --- a/python/lean_pool/rebase.py +++ b/python/lean_pool/rebase.py @@ -29,13 +29,14 @@ import sys from pathlib import Path +import yaml + from lean_pool.exposition.source_text import code_view logger = logging.getLogger(__name__) REGISTRY = "LeanPool/projects.yml" INDEX = "LeanPool.lean" -CARD_PREFIX = " - slug: " # The only conflicts this module claims to resolve. RESOLVABLE = frozenset({INDEX, REGISTRY}) @@ -78,22 +79,57 @@ def render_index(root: Path) -> str: ) +def _project_nodes(text: str) -> list[yaml.MappingNode]: + """Read card boundaries from YAML rather than assuming a first key.""" + root = yaml.compose(text, Loader=yaml.SafeLoader) + if not isinstance(root, yaml.MappingNode) or len(root.value) != 1: + raise ValueError("Expected a registry containing only projects") + key, projects = root.value[0] + if key.value != "projects" or not isinstance(projects, yaml.SequenceNode): + raise ValueError("Expected projects to be a sequence") + if projects.flow_style and projects.value: + raise ValueError("Project cards must use a block sequence") + if any(not isinstance(card, yaml.MappingNode) for card in projects.value): + raise ValueError("Each project card must be a mapping") + return projects.value + + +def _card_slug(card: yaml.MappingNode) -> str: + """Require one unambiguous string slug, in any mapping position.""" + slugs = [value for key, value in card.value if key.value == "slug"] + if ( + len(slugs) != 1 + or not isinstance(slugs[0], yaml.ScalarNode) + or slugs[0].tag != "tag:yaml.org,2002:str" + or not slugs[0].value.strip() + ): + raise ValueError("Each project card must have exactly one nonempty string slug") + return slugs[0].value + + def split_cards(text: str) -> tuple[str, list[tuple[str, str]]]: """Split a registry into its header and its cards, as verbatim text. Returns ``(header, [(slug, block)])`` where concatenating the header and every block reproduces ``text`` exactly. """ - lines = text.splitlines(keepends=True) - starts = [i for i, line in enumerate(lines) if line.startswith(CARD_PREFIX)] + nodes = _project_nodes(text) + starts = [text.rfind("\n", 0, card.start_mark.index) + 1 for card in nodes] if not starts: return text, [] - header = "".join(lines[: starts[0]]) + if starts != sorted(set(starts)) or any( + not re.fullmatch(r"[ \t]*-[ \t]+", text[start : node.start_mark.index]) + for start, node in zip(starts, nodes, strict=True) + ): + raise ValueError("Each project card must start on its own sequence-item line") + header = text[: starts[0]] cards: list[tuple[str, str]] = [] - for index, start in enumerate(starts): - end = starts[index + 1] if index + 1 < len(starts) else len(lines) - slug = lines[start][len(CARD_PREFIX) :].strip() - cards.append((slug, "".join(lines[start:end]))) + for index, (start, node) in enumerate(zip(starts, nodes, strict=True)): + end = starts[index + 1] if index + 1 < len(starts) else len(text) + slug = _card_slug(node) + if any(existing == slug for existing, _ in cards): + raise ValueError(f"Duplicate project slug: {slug}") + cards.append((slug, text[start:end])) return header, cards @@ -122,7 +158,10 @@ def merge_registry(base: str, ours: str, theirs: str) -> str: # run into the first appended card. if merged and not merged.endswith("\n"): merged += "\n" - return merged + "".join(block for _, block in added) + result = merged + "".join(block for _, block in added) + # Reject incompatible layouts instead of pushing a malformed registry. + split_cards(result) + return result def resolvable(conflicts: list[str]) -> bool: diff --git a/python/tests/test_rebase.py b/python/tests/test_rebase.py index cebda23e24..70ad2a3ce1 100644 --- a/python/tests/test_rebase.py +++ b/python/tests/test_rebase.py @@ -5,6 +5,7 @@ from pathlib import Path import pytest +import yaml from lean_pool.rebase import ( main, @@ -155,6 +156,75 @@ def test_split_cards_round_trips() -> None: assert header + "".join(block for _, block in cards) == OURS +@pytest.mark.parametrize("indent", ["", " ", " "]) +def test_split_cards_uses_yaml_boundaries_and_decoded_slugs(indent: str) -> None: + """Key order, quoting, comments and nested lists do not change card identity.""" + text = ( + "# Keep this header\nprojects:\n" + f'{indent}- title: "First: project"\n' + f"{indent} summary: |\n{indent} - slug: not-a-card\n" + f"{indent} tags:\n{indent} - slug: nested\n" + f"{indent} slug: 'alpha' # Keep this comment\n" + f'{indent}- slug: "beta"\n{indent} title: Beta\n' + ) + header, cards = split_cards(text) + assert [slug for slug, _ in cards] == ["alpha", "beta"] + assert header + "".join(block for _, block in cards) == text + + +def test_merge_preserves_title_first_added_cards_verbatim() -> None: + """Regression: the automatic updater must not silently drop a title-first card.""" + added = " - title: Delta\n slug: 'delta' # quoted identity\n" + merged = merge_registry(BASE, OURS, BASE + added) + assert merged == OURS + added + assert [card["slug"] for card in yaml.safe_load(merged)["projects"]] == [ + "alpha", + "beta", + "gamma", + "delta", + ] + + +def test_merge_recognizes_title_first_base_and_main_cards() -> None: + """Reordered keys and quoted slugs must not duplicate existing cards.""" + base = "projects:\n - title: Alpha\n slug: 'alpha'\n" + ours = base + ' - title: Delta\n slug: "delta"\n' + theirs = "projects:\n - slug: alpha\n title: Alpha\n" + theirs += " - slug: delta\n title: Delta\n" + assert merge_registry(base, ours, theirs) == ours + + +@pytest.mark.parametrize( + "text", + [ + "projects:\n - title: Missing slug\n", + "projects:\n - slug: alpha\n slug: beta\n", + "projects:\n - slug: alpha\n - slug: 'alpha'\n", + "projects:\n - slug: 123\n", + "projects:\n - slug: ''\n", + "projects: [{slug: alpha}, {slug: beta}]\n", + "projects:\n - &card {slug: alpha}\n - *card\n", + "projects:\n - not-a-mapping\n", + "projects: {}\n", + ], +) +def test_split_cards_rejects_ambiguous_registries(text: str) -> None: + """Malformed identities and unsupported layouts fail before anything is pushed.""" + with pytest.raises(ValueError): + split_cards(text) + + +def test_split_cards_covers_every_real_project() -> None: + """Every YAML project, including title-first entries, gets a verbatim block.""" + root = Path(__file__).resolve().parents[2] + text = (root / "LeanPool/projects.yml").read_text(encoding="utf-8") + header, cards = split_cards(text) + assert [slug for slug, _ in cards] == [ + card["slug"] for card in yaml.safe_load(text)["projects"] + ] + assert header + "".join(block for _, block in cards) == text + + def test_merge_keeps_both_additions() -> None: """The merged registry has the card main added and the card we added.""" merged = merge_registry(BASE, OURS, THEIRS) From 154ab03d55658b1dd6ddb4d19f3d5ff04614dd62 Mon Sep 17 00:00:00 2001 From: Vasily Ilin Date: Fri, 25 Sep 2026 20:25:01 -0700 Subject: [PATCH 2/2] fix: recognize standalone YAML sequence markers --- python/lean_pool/rebase.py | 22 +++++++++++++--------- python/tests/test_rebase.py | 8 ++++++++ 2 files changed, 21 insertions(+), 9 deletions(-) diff --git a/python/lean_pool/rebase.py b/python/lean_pool/rebase.py index 51165f8fb4..7594115af7 100644 --- a/python/lean_pool/rebase.py +++ b/python/lean_pool/rebase.py @@ -79,7 +79,7 @@ def render_index(root: Path) -> str: ) -def _project_nodes(text: str) -> list[yaml.MappingNode]: +def _project_nodes(text: str) -> yaml.SequenceNode: """Read card boundaries from YAML rather than assuming a first key.""" root = yaml.compose(text, Loader=yaml.SafeLoader) if not isinstance(root, yaml.MappingNode) or len(root.value) != 1: @@ -91,7 +91,7 @@ def _project_nodes(text: str) -> list[yaml.MappingNode]: raise ValueError("Project cards must use a block sequence") if any(not isinstance(card, yaml.MappingNode) for card in projects.value): raise ValueError("Each project card must be a mapping") - return projects.value + return projects def _card_slug(card: yaml.MappingNode) -> str: @@ -113,15 +113,19 @@ def split_cards(text: str) -> tuple[str, list[tuple[str, str]]]: Returns ``(header, [(slug, block)])`` where concatenating the header and every block reproduces ``text`` exactly. """ - nodes = _project_nodes(text) - starts = [text.rfind("\n", 0, card.start_mark.index) + 1 for card in nodes] + projects = _project_nodes(text) + nodes = projects.value + entries = [ + token + for token in yaml.scan(text, Loader=yaml.SafeLoader) + if isinstance(token, yaml.tokens.BlockEntryToken) + and token.start_mark.column == projects.start_mark.column + ] + starts = [text.rfind("\n", 0, token.start_mark.index) + 1 for token in entries] + if len(starts) != len(nodes): + raise ValueError("Expected one sequence item per project card") if not starts: return text, [] - if starts != sorted(set(starts)) or any( - not re.fullmatch(r"[ \t]*-[ \t]+", text[start : node.start_mark.index]) - for start, node in zip(starts, nodes, strict=True) - ): - raise ValueError("Each project card must start on its own sequence-item line") header = text[: starts[0]] cards: list[tuple[str, str]] = [] for index, (start, node) in enumerate(zip(starts, nodes, strict=True)): diff --git a/python/tests/test_rebase.py b/python/tests/test_rebase.py index 70ad2a3ce1..074765a92e 100644 --- a/python/tests/test_rebase.py +++ b/python/tests/test_rebase.py @@ -185,6 +185,14 @@ def test_merge_preserves_title_first_added_cards_verbatim() -> None: ] +def test_merge_preserves_cards_with_keys_below_the_dash() -> None: + """A standalone YAML sequence marker is a valid card boundary too.""" + added = " - # project comment\n title: Delta\n slug: delta\n" + merged = merge_registry(BASE, OURS, BASE + added) + assert merged == OURS + added + assert yaml.safe_load(merged)["projects"][-1]["slug"] == "delta" + + def test_merge_recognizes_title_first_base_and_main_cards() -> None: """Reordered keys and quoted slugs must not duplicate existing cards.""" base = "projects:\n - title: Alpha\n slug: 'alpha'\n"