Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
35 changes: 30 additions & 5 deletions scripts/translation/check_pair_count_declared.py
Original file line number Diff line number Diff line change
Expand Up @@ -136,12 +136,22 @@ def constant_modified(self) -> bool:
"""
return self.constant_change.old_value != self.constant_change.new_value

@property
def count_changing_pairs(self) -> list["PairChange"]:
"""Pairs whose add/remove shifts the declared count. A pure rename
(an existing translation file renamed alongside its source) keeps
the pair count identical, so it must not require a bump -- before
this distinction, no honest constant value could satisfy the guard
on a rename (see #21391, reproduction on PRs #22932/#22670).
"""
return [pc for pc in self.pair_changes if pc.status != "renamed"]

@property
def rouge(self) -> bool:
if not self.has_pair_change and not self.constant_modified:
return False
# pair added/removed WITHOUT a constant bump = rouge
if self.has_pair_change and not self.constant_modified:
if self.count_changing_pairs and not self.constant_modified:
return True
# constant modified but value doesn't match the actual count = rouge
if self.constant_modified and self.actual_count is not None:
Expand Down Expand Up @@ -280,7 +290,7 @@ def check_pair_count_declared(
report = CheckReport(base=base, head=head)

rows = _git_diff_name_status(base, head, path_list, repo_root=repo_root)
for status, new, _old in rows:
for status, new, old in rows:
if status not in ("A", "D", "R"):
continue
info = _classify_translation(new)
Expand All @@ -304,8 +314,19 @@ def check_pair_count_declared(
# A or R: check the source exists at head
if not _pair_at_head(head, new, repo_root=repo_root):
continue
# An R whose OLD path was itself a translation file is a pure
# rename (same pair under a new name): the pair count is
# preserved. An R whose old path was NOT a translation (a
# non-notebook renamed into a *_<lang>.ipynb path) creates a
# pair where none existed: classify it as an addition so the
# bump requirement still applies.
is_pure_rename = (
status == "R"
and old is not None
and _classify_translation(old) is not None
)
report.pair_changes.append(PairChange(
status="added" if status == "A" else "renamed",
status="renamed" if is_pure_rename else "added",
translation_path=Path(new).as_posix(),
source_path=Path(src_path).as_posix(),
stem=stem, lang=lang,
Expand Down Expand Up @@ -359,7 +380,9 @@ def _rouge_message(report: CheckReport, repo_root: Path | None = None) -> str:
if report.pair_changes and not report.constant_modified:
# Compute the next count (assume base + adds - removes)
old = report.constant_change.old_value or 0
delta_added = sum(1 for pc in report.pair_changes if pc.status in ("added", "renamed"))
# Only add/remove shifts the declared count; a pure rename
# (status "renamed") preserves it and requires no bump.
delta_added = sum(1 for pc in report.pair_changes if pc.status == "added")
delta_removed = sum(1 for pc in report.pair_changes if pc.status == "removed")
next_value = old + delta_added - delta_removed
# Derive the constant line number at the head we inspected -- never
Expand Down Expand Up @@ -399,8 +422,10 @@ def render_text(report: CheckReport, repo_root: Path | None = None) -> str:
if not report.pair_changes and not report.constant_modified:
return "GREEN: no pair change, no constant bump -- nothing to check."
if not report.rouge:
renamed = sum(1 for pc in report.pair_changes if pc.status == "renamed")
rename_note = f" ({renamed} pure rename(s), count preserved)" if renamed else ""
return (
f"GREEN: {len(report.pair_changes)} pair change(s), constant bumped "
f"GREEN: {len(report.pair_changes)} pair change(s){rename_note}, constant "
f"{report.constant_change.old_value} -> {report.constant_change.new_value}, "
f"actual count = {report.actual_count}."
)
Expand Down
64 changes: 64 additions & 0 deletions scripts/translation/tests/test_check_pair_count_declared.py
Original file line number Diff line number Diff line change
Expand Up @@ -324,6 +324,70 @@ def test_renamed_pair_no_constant_change(self):
# at head -- no pair change recorded.
assert r.rouge is False or len(r.pair_changes) == 0

def test_full_pair_rename_is_green_without_bump(self):
# #21391 reproduction (PRs #22932/#22670): the FR source AND its
# _en translation renamed together (R status on both). The pair
# count is preserved, so no honest constant value satisfied the
# guard before the fix: N -> N is "not modified", N -> M is a
# wrong count. The guard must stay green and report the pair as
# renamed, not added.
repo = _make_repo()
_reset(repo)
base = _add_pair(repo, "Foo")
nb = repo / "MyIA.AI.Notebooks" / "Translation"
(nb / "Foo.ipynb").rename(nb / "Bar.ipynb")
(nb / "Foo_en.ipynb").rename(nb / "Bar_en.ipynb")
subprocess.run(["git", "add", "-A"], check=True, cwd=repo)
subprocess.run(["git", "commit", "-q", "-m", "rename pair Foo -> Bar"],
check=True, cwd=repo)
head = subprocess.run(
["git", "rev-parse", "HEAD"], capture_output=True, text=True, encoding="utf-8",
cwd=repo,
).stdout.strip()
r = check_pair_count_declared(base, head, repo_root=repo, do_disk_count=False)
# The pair is reported, classified "renamed" (not added)...
assert len(r.pair_changes) == 1
assert r.pair_changes[0].status == "renamed"
assert r.pair_changes[0].stem == "Bar"
# ...the constant is untouched (0 -> 0, not modified)...
assert r.constant_modified is False
# ...and the verdict is GREEN: a pure rename does not drift the count.
assert r.rouge is False

def test_rename_into_translation_still_requires_bump(self):
# Guard the carve-out from the other side: renaming a NON-translation
# file into a *_en.ipynb path (old path not a translation, e.g.
# notes.md -> Bar_en.ipynb) creates a pair where none existed. It
# must stay classified "added" so the bump requirement still applies.
repo = _make_repo()
_reset(repo)
scratch = repo / "notes.md"
scratch.write_text("# shared content\n")
subprocess.run(["git", "add", "-A"], check=True, cwd=repo)
subprocess.run(["git", "commit", "-q", "-m", "add scratch"], check=True, cwd=repo)
base = subprocess.run(
["git", "rev-parse", "HEAD"], capture_output=True, text=True, encoding="utf-8",
cwd=repo,
).stdout.strip()
nb = repo / "MyIA.AI.Notebooks" / "Translation"
nb.mkdir(parents=True, exist_ok=True)
(nb / "Bar.ipynb").write_text("# FR source for Bar\n")
scratch.rename(nb / "Bar_en.ipynb") # same content -> R100
subprocess.run(["git", "add", "-A"], check=True, cwd=repo)
subprocess.run(["git", "commit", "-q", "-m", "rename notes.md into Bar_en"],
check=True, cwd=repo)
head = subprocess.run(
["git", "rev-parse", "HEAD"], capture_output=True, text=True, encoding="utf-8",
cwd=repo,
).stdout.strip()
r = check_pair_count_declared(base, head, repo_root=repo, do_disk_count=False)
# Classified "added" (the old path was not a translation), so the
# bump is still required -- rouge without it.
assert len(r.pair_changes) == 1
assert r.pair_changes[0].status == "added"
assert r.constant_modified is False
assert r.rouge is True

def test_constant_path_does_not_drift(self):
# The constant is at scripts/translation/tests/test_check_translation_parity.py
# -- guard against accidental path move.
Expand Down
Loading