Skip to content

Commit dce4972

Browse files
fix(images): disambiguate relative images that share a basename (#160)
Two relative source images with the same basename but different paths (e.g. a/logo.png and b/logo.png) overwrote each other in sources/images/<doc>/, collapsing both markdown links onto one file. Track the destination assigned to each source (so an image referenced twice is copied once) and suffix genuine basename collisions (logo.png -> logo_1.png). Adapted from #122 by @jichaowang02-lang; drops that PR's second commit, which seeded the taken-name set from the existing images_dir and thereby broke re-convert idempotency (a changed same-basename image got a fresh suffix each run, orphaning the old file and churning links). Claude-Session: https://claude.ai/code/session_01UtbmJxjtw6FtP8fUXUKVtg Co-authored-by: jichao wang <jichaowang02@gmail.com>
1 parent 4d9319d commit dce4972

2 files changed

Lines changed: 53 additions & 4 deletions

File tree

openkb/images.py

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,13 @@ def copy_relative_images(markdown: str, source_dir: Path, doc_name: str, images_
224224
- Missing source file: log a warning and leave the original text unchanged.
225225
"""
226226
result = markdown
227+
# Track the destination chosen for each already-copied source so the same
228+
# image referenced twice isn't duplicated, plus the set of taken names so
229+
# two *different* sources that share a basename (e.g. ``a/logo.png`` and
230+
# ``b/logo.png``) don't overwrite each other and collapse both links onto a
231+
# single image.
232+
assigned: dict[Path, str] = {}
233+
taken: set[str] = set()
227234

228235
for match in _RELATIVE_RE.finditer(markdown):
229236
alt, rel_path = match.group(1), match.group(2)
@@ -235,10 +242,17 @@ def copy_relative_images(markdown: str, source_dir: Path, doc_name: str, images_
235242
logger.warning("Relative image not found: %s; leaving original link.", src)
236243
continue
237244

238-
filename = src.name
239-
dest = images_dir / filename
240-
images_dir.mkdir(parents=True, exist_ok=True)
241-
shutil.copy2(src, dest)
245+
filename = assigned.get(src)
246+
if filename is None:
247+
filename = src.name
248+
n = 1
249+
while filename in taken:
250+
filename = f"{src.stem}_{n}{src.suffix}"
251+
n += 1
252+
assigned[src] = filename
253+
taken.add(filename)
254+
images_dir.mkdir(parents=True, exist_ok=True)
255+
shutil.copy2(src, images_dir / filename)
242256

243257
new_ref = f"![{alt}](sources/images/{doc_name}/{filename})"
244258
result = result.replace(match.group(0), new_ref, 1)

tests/test_images.py

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,3 +161,38 @@ def test_multiple_relative_images_all_copied(self, tmp_path):
161161
assert "![b](sources/images/doc/b.jpg)" in result
162162
assert (images_dir / "a.png").exists()
163163
assert (images_dir / "b.jpg").exists()
164+
165+
def test_same_basename_different_dirs_no_overwrite(self, tmp_path):
166+
# Two distinct images sharing a basename must not overwrite each other
167+
# (which would lose one image and point both links at the survivor).
168+
source_dir = tmp_path / "source"
169+
(source_dir / "a").mkdir(parents=True)
170+
(source_dir / "b").mkdir(parents=True)
171+
(source_dir / "a" / "logo.png").write_bytes(FAKE_PNG)
172+
(source_dir / "b" / "logo.png").write_bytes(FAKE_JPG)
173+
174+
images_dir = tmp_path / "images" / "doc"
175+
images_dir.mkdir(parents=True)
176+
177+
md = "![a](a/logo.png)\n![b](b/logo.png)"
178+
result = copy_relative_images(md, source_dir, "doc", images_dir)
179+
180+
saved = sorted(p.name for p in images_dir.iterdir())
181+
assert len(saved) == 2 # both copied, neither overwritten
182+
assert {(images_dir / n).read_bytes() for n in saved} == {FAKE_PNG, FAKE_JPG}
183+
links = sorted(line.split("](")[1].rstrip(")") for line in result.strip().splitlines())
184+
assert links[0] != links[1] # links point at different files
185+
186+
def test_same_image_referenced_twice_is_copied_once(self, tmp_path):
187+
# Identical source referenced twice: copy once, both links agree.
188+
source_dir = tmp_path / "source"
189+
source_dir.mkdir()
190+
(source_dir / "logo.png").write_bytes(FAKE_PNG)
191+
images_dir = tmp_path / "images" / "doc"
192+
images_dir.mkdir(parents=True)
193+
194+
md = "![x](logo.png)\n![y](logo.png)"
195+
result = copy_relative_images(md, source_dir, "doc", images_dir)
196+
197+
assert [p.name for p in images_dir.iterdir()] == ["logo.png"]
198+
assert result.count("sources/images/doc/logo.png") == 2

0 commit comments

Comments
 (0)