From 5919c9b420f87bd70d4864df7c22d5dbf52193ab Mon Sep 17 00:00:00 2001 From: tya5 <370909+tya5@users.noreply.github.com> Date: Wed, 12 Aug 2026 03:03:08 +0900 Subject: [PATCH] fix(#4295): absorb a gap blank line between two chunks the same rewrite removes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Found via real usage (lead-coder's dogfood-verification request): tui-coder's own un-migrated reyn.yaml had model: and models: separated by exactly one blank line. Migrating both into a new llm: block produced TWO blank lines before the next section — the gap blank line between the two OLD keys was claimed by NEITHER removal (model's is a single-line scalar with no block, models' own block-extent absorbs only its OWN trailing blank), so it survived unclaimed and collided with the moved block's own trailing blank. migrate_text.py's promise is "byte-for-byte untouched except the moved keys" — a gap strictly BETWEEN two keys this same rewrite is removing is touched by definition once both keys are gone (lead-coder: the promise is "don't touch", not "don't lose data" — an extra blank line is a real violation of the former even though it loses nothing). Fix: after computing each key's removed line-range, merge any two ranges from the SAME rewrite that are separated only by blank lines into one contiguous removed range, so the gap blank line is absorbed along with the keys it used to separate. Test: test_migrate_does_not_leave_a_doubled_blank_line_between_merged_chunks pins line-count identity around the merged llm: block — exactly one blank line survives where the original had one between models: and the next section, not two. Falsify-verified: reverted the merge fix, confirmed the test goes RED (two blank lines observed, matching the real bug), restored, confirmed green (18/18 in the file). Verified: ruff check, mypy_ratchet, test_tier_audit --strict all clean; tests/interfaces/test_config_validate_migrate_command_4174.py (18 passed). part of #4295 Co-Authored-By: Claude Sonnet 5 --- src/reyn/config/migrate_text.py | 18 +++++ ...st_config_validate_migrate_command_4174.py | 67 +++++++++++++++++++ 2 files changed, 85 insertions(+) diff --git a/src/reyn/config/migrate_text.py b/src/reyn/config/migrate_text.py index 19dbf2d39..68711b2a7 100644 --- a/src/reyn/config/migrate_text.py +++ b/src/reyn/config/migrate_text.py @@ -203,6 +203,24 @@ def rewrite_text(text: str, renames: dict[str, str]) -> RewriteResult: (extractions[old_key][1], extractions[old_key][2]) for old_key in extractions ] + # A blank line that sits BETWEEN two chunks this SAME rewrite is + # already removing (and nothing else) was only ever separating those + # two keys from each other — once both are gone, that separator has + # nothing left to separate. Absorb it into the removal too, or it + # survives as an orphaned blank line and collides with whatever + # trailing blank the merged/inserted block ends with (byte-for-byte + # "untouched" only holds for what this rewrite doesn't touch AT ALL; + # a gap strictly between two touched keys is touched by definition). + removed_ranges.sort() + merged_ranges: list[list[int]] = [] + for start, end in removed_ranges: + if merged_ranges and all( + _BLANK_RE.match(lines[i]) for i in range(merged_ranges[-1][1], start) + ): + merged_ranges[-1][1] = end + else: + merged_ranges.append([start, end]) + removed_ranges = [(s, e) for s, e in merged_ranges] # First pass: does `parent` already exist as a top-level key in the # ORIGINAL text (not counting what we're about to remove — a renamed diff --git a/tests/interfaces/test_config_validate_migrate_command_4174.py b/tests/interfaces/test_config_validate_migrate_command_4174.py index 21681327b..da9fa1dc5 100644 --- a/tests/interfaces/test_config_validate_migrate_command_4174.py +++ b/tests/interfaces/test_config_validate_migrate_command_4174.py @@ -512,3 +512,70 @@ def _flatten(d, prefix="") -> dict: assert after_flat["llm.api_base"] == "http://localhost:8000" assert after_flat["audit_events.keep_days"] == 30 assert after_flat["unrelated_key"] == [1, 2, 3] + + +def test_migrate_does_not_leave_a_doubled_blank_line_between_merged_chunks( + project, monkeypatch, +) -> None: + """Tier 2: #4295 follow-up — a blank line sitting BETWEEN two chunks + the SAME migrate run removes (and nothing else) must be absorbed into + the removal, not left as an orphaned separator. + + Found via real usage (lead-coder's dogfood-verification request): + tui-coder's own un-migrated `reyn.yaml` had `model:` and `models:` + separated by exactly one blank line; after migrating both into a new + `llm:` block, the output had TWO blank lines before the next section + — the blank line between the two OLD keys was neither claimed by + `model`'s own (single-line, no-block) removal nor by `models`'s + (whose own block-value removal already absorbs its OWN trailing + blank, per `_block_extent`), so it survived unclaimed and collided + with the moved block's own trailing blank. `migrate_text.py`'s + promise is "byte-for-byte untouched except the moved keys" — a gap + strictly BETWEEN two touched keys is touched by definition once both + keys are gone; leaving an extra blank line there is a real + (if data-loss-free) violation of that promise, not cosmetic noise to + shrug off. + + Pins line-COUNT identity, not just "does it look right": migrate must + not add or drop a single line beyond the mechanical rename+reindent + the two moved keys' own content requires.""" + from reyn.config.config_schema import RenamedKeyHint + + monkeypatch.setattr( + "reyn.config.config_schema._RENAMED_CONFIG_KEYS", + { + "model": RenamedKeyHint(note="-> llm.model", destination="llm.model"), + "models": RenamedKeyHint(note="-> llm.models", destination="llm.models"), + }, + ) + original_text = ( + "# Reyn project configuration\n" + "# Committed to git.\n" + "\n" + "# Default model class when --model is not specified\n" + "model: standard\n" + "\n" + "# Model class -> LiteLLM model string\n" + "# Three standard tiers.\n" + "models:\n" + " light: gemini-flash-lite\n" + " standard: gemini-flash-lite\n" + " strong: gemini-pro\n" + "\n" + "# Project-wide context injected into the system prompt.\n" + ) + _write_yaml(project / "reyn.yaml", original_text) + + from reyn.interfaces.cli.commands.config import _migrate + _migrate() + + text = (project / "reyn.yaml").read_text() + lines = text.split("\n") + # Exactly one blank line between the moved `models:` block's last + # sub-key and the next real section — not two. + strong_idx = next(i for i, ln in enumerate(lines) if "strong:" in ln) + assert lines[strong_idx + 1] == "", "expected a blank line right after the models: block" + assert lines[strong_idx + 2] != "", ( + f"a second, orphaned blank line survived: {lines[strong_idx:strong_idx + 4]!r}" + ) + assert "# Project-wide context injected into the system prompt." in lines[strong_idx + 2]