Skip to content

Commit 2146c03

Browse files
committed
v0.2.5: Fix organize silently overwriting files with duplicate names
execute_plan() computed its destination as `cat_dir / new_name` with no existence check and copied with shutil.copy2(), which overwrites. When the AI plan assigned the same category + new_name to two different files, only one survived on disk -- while stats["copied"] counted both and _summary.md listed both as organized successfully. Silent data loss with a success report. Dedup masks this in practice: compare --link converts identical content to symlinks before organize runs, so byte-identical files rarely collide. The exposure is genuinely different content earning the same name -- two unrelated shopping lists both becoming shopping-list.txt. The defect has been present since organize was introduced. Found while designing batched AI backends, where it becomes much more likely: a backend processing files in batches has no view of the names earlier batches already assigned. Collision handling: - _resolve_unique_dest() claims a destination no other entry in the same run is using, appending -2, -3, ... before the extension so file type is preserved (script.py -> script-2.py) - Keys on the lowercased path, so Notes.txt and notes.txt are treated as a collision on Windows rather than one silently winning - Tracking is scoped to a single run, never checked against files already on disk. Checking the filesystem would break idempotency: re-running organize would produce -2, -3 copies on every pass instead of overwriting its own previous output - Every rename is reported in three places -- the summary counts, a new "Renamed to avoid collisions" section in _summary.md, and the per-file mapping. A silent rename is nearly as bad as a silent overwrite, because the summary would stop describing the disk - stats["renamed"] added and surfaced in the CLI output Policy is auto-rename and report, not abort. One name clash must not discard an otherwise complete run; the invariant that matters is that no file is lost, not that no name repeats. Plan validation: - validate_plan(plan, base_dir, expected_sources=None) checks a plan before anything touches the filesystem: every source must exist, no source may appear twice, every expected source must be present, and duplicate destinations are reported - organize now refuses to run when a plan would lose or mis-file content. Duplicate destinations are informational (execute_plan handles them); missing, duplicated, and nonexistent sources are fatal Prompt: - prompts/organize.md had no uniqueness requirement at all. "Every file MUST get its own new_name" meant "not null", as the following clause says. Added an explicit rule that every category + new_name pair must be unique, and that similar files be distinguished by content rather than by numeric suffix New tests (50 total, up from 36) -- first automated suite in tests/, which previously held only one-offs/ proof-of-concept scripts: - test_colliding_names_do_not_lose_content: reproduces the defect; two entries sharing a destination must yield two files with both contents - test_collision_rename_is_reported: renames appear in the summary - test_collision_across_categories_is_not_a_collision: the same filename in different categories is legitimate and must not be renamed - test_extension_preserved_when_de_colliding: script.py -> script-2.py - test_case_only_difference_collides_on_windows: Notes.txt vs notes.txt - test_rerun_does_not_accumulate_duplicates: idempotency guard - test_many_collisions_all_survive: a 12-way collision keeps all 12 files - test_rename_counter_is_reported_in_stats - test_validate_plan_*: duplicate, missing, and nonexistent sources - no-regression guards on the ordinary non-colliding path Also adds tests/checklists/ with a human test checklist covering what mocks cannot: whether _summary.md reads clearly, real Claude CLI output, Windows case-insensitivity, whether the refusal message is actionable, and re-run idempotency against a real organized folder. Design: 2026-07-25__14-07-53__editor-mode-architecture-and-sublime-archaeology.md Design: 2026-07-26__03-15-17__addendum__local-model-backend-and-collision-defect.md
1 parent 450e79e commit 2146c03

7 files changed

Lines changed: 759 additions & 8 deletions

File tree

CHANGELOG.md

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,52 @@
22

33
All notable changes to notepad-cleanup will be documented in this file.
44

5+
## [0.2.5] - 2026-07-26
6+
7+
### Fixed
8+
- **`organize` no longer silently overwrites files when two get the same name.**
9+
`execute_plan()` computed the destination path with no existence check and used
10+
`shutil.copy2()`, which overwrites. When the AI plan assigned the same
11+
`category` + `new_name` to two different files, only one survived on disk --
12+
while `stats["copied"]` counted both and `_summary.md` reported both as
13+
organized successfully. Silent data loss with a success report
14+
- Colliding destinations are now de-collided (`name.txt` -> `name-2.txt`), keeping
15+
every file, and each rename is reported in three places: the summary counts, a
16+
dedicated "Renamed to avoid collisions" section, and the per-file mapping.
17+
Policy is auto-rename and report -- never silent, and never abort a whole run
18+
over a name clash, because the invariant that matters is *no file is lost*
19+
- Collision tracking is scoped to a single run, so re-running `organize` still
20+
overwrites its own previous output rather than accumulating `-2`, `-3` copies
21+
22+
### Added
23+
- `validate_plan(plan, base_dir, expected_sources=None)` -- checks a plan before
24+
any file is touched: sources must exist, no source may appear twice, every
25+
expected source must be present, and duplicate destinations are flagged.
26+
`organize` now refuses to run when a plan would lose or mis-file content
27+
- `stats["renamed"]` counts collision renames, surfaced in the CLI output
28+
- Uniqueness rules in `prompts/organize.md`: every `category` + `new_name` pair
29+
must be unique, and similar files must be distinguished by content rather than
30+
by numeric suffix. The prompt previously had no uniqueness requirement at all
31+
32+
### Tests
33+
- `tests/test_organize_collisions.py` -- first automated regression suite in
34+
`tests/` (previously only `tests/one-offs/` POC scripts existed):
35+
- `test_colliding_names_do_not_lose_content`: reproduces the defect; two
36+
entries sharing a destination must yield two files with both contents
37+
- `test_collision_rename_is_reported`: renames must appear in the summary
38+
- `test_collision_across_categories_is_not_a_collision`: same filename in
39+
different categories is legitimate and must not be renamed
40+
- `test_extension_preserved_when_de_colliding`: `script.py` -> `script-2.py`
41+
- `test_many_collisions_all_survive`: 12-way collision keeps all 12 files
42+
- `test_case_only_difference_collides_on_windows`: `Notes.txt` vs `notes.txt`
43+
collide on a case-insensitive filesystem; both must survive
44+
- `test_rerun_does_not_accumulate_duplicates`: idempotency guard -- a second
45+
`organize` run must overwrite its own output, not create `-2` copies
46+
- `test_rename_counter_is_reported_in_stats`: `stats['renamed']` is accurate
47+
- `test_validate_plan_*`: duplicate, missing, and nonexistent sources
48+
- no-regression guards on the ordinary non-colliding path
49+
- suite total: 14 new tests; **50 passed** overall (was 36)
50+
551
## [0.2.4] - 2026-04-10
652

753
### Fixed

notepad_cleanup/_version.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,13 +21,13 @@
2121
# Version components - edit these for version bumps
2222
MAJOR = 0
2323
MINOR = 2
24-
PATCH = 4
24+
PATCH = 5
2525
PHASE = None # Per-MINOR feature set: None, "alpha", "beta", "rc1", etc.
2626
PRE_RELEASE_NUM = 0 # PEP 440 pre-release number (e.g., a1, b2)
2727
PROJECT_PHASE = "prealpha" # Project-wide: "prealpha", "alpha", "beta", "stable"
2828

2929
# Auto-updated by git hooks - do not edit manually
30-
__version__ = "0.2.4_main_16-20260410-34ed8c9d"
30+
__version__ = "0.2.5_main_16-20260726-450e79e2"
3131
__app_name__ = "notepad-cleanup"
3232

3333

notepad_cleanup/cli.py

Lines changed: 21 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
from .extractor import extract_phase1, extract_phase2, merge_results
1414
from .saver import save_extraction
1515
from .organizer import (generate_prompt, invoke_claude_cli, save_prompt_to_file,
16-
find_claude_cli, parse_plan, execute_plan,
16+
find_claude_cli, parse_plan, execute_plan, validate_plan,
1717
separate_links, join_links)
1818
from .dedup import (find_session_dirs, build_hash_index, find_duplicates,
1919
find_content_files,
@@ -614,6 +614,23 @@ def organize(folder, last, backend, dry_run, verbose):
614614

615615
console.print(f" Claude proposed a plan for [bold]{len(plan)}[/bold] files\n")
616616

617+
# Step 2b: Validate the plan before touching the filesystem. Duplicate or
618+
# missing sources mean content would be mis-filed or left unorganized --
619+
# which matters most because later steps may delete the origin.
620+
problems = validate_plan(plan, folder)
621+
if problems:
622+
fatal = [p for p in problems if "auto-renamed" not in p]
623+
for p in problems:
624+
style = "yellow" if "auto-renamed" in p else "red"
625+
console.print(f" [{style}]plan issue:[/{style}] {p}")
626+
console.print("")
627+
if fatal:
628+
console.print(
629+
"[red]Refusing to organize: the plan has problems that would "
630+
"lose or mis-file content.[/red]")
631+
console.print(f" Raw output saved to: {log_file}\n")
632+
return
633+
617634
# Step 3: Execute the plan locally (link-aware)
618635
with console.status("Organizing files..."):
619636
summary, stats = execute_plan(plan, folder, linked_paths=linked_paths)
@@ -622,6 +639,9 @@ def organize(folder, last, backend, dry_run, verbose):
622639
console.print(f" Files copied: {stats['copied']}")
623640
if stats.get('linked', 0):
624641
console.print(f" Files linked: {stats['linked']} (symlink/hardlink to canonical)")
642+
if stats.get('renamed', 0):
643+
console.print(f" [yellow]Renamed: {stats['renamed']} "
644+
f"(name collisions -- see _summary.md)[/yellow]")
625645
if stats['errors']:
626646
console.print(f" [red]Errors: {stats['errors']}[/red]")
627647

notepad_cleanup/organizer.py

Lines changed: 134 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -304,6 +304,108 @@ def _write_organized_link_manifest(entries, organized_dir):
304304
)
305305

306306

307+
def _resolve_unique_dest(cat_dir, new_name, used):
308+
"""Pick a destination path that no other entry in THIS run has claimed.
309+
310+
Two plan entries can legitimately propose the same `new_name` -- an AI
311+
backend batching files has no global view, and even a single-shot backend
312+
may name two different grocery lists "grocery-list.txt". Copying both to
313+
the same path silently destroys the first one, so we de-collide instead.
314+
315+
Scope note: collisions are tracked only WITHIN a run (via `used`), never
316+
against files already on disk. Checking the filesystem would break
317+
idempotency -- re-running organize would produce name-2, name-3, ... on
318+
every pass instead of overwriting its own previous output.
319+
320+
Returns: (dest_path, final_name, original_name_if_renamed)
321+
"""
322+
stem = Path(new_name).stem
323+
suffix = Path(new_name).suffix
324+
325+
candidate = new_name
326+
n = 1
327+
while True:
328+
dest = cat_dir / candidate
329+
key = str(dest).lower() # Windows paths are case-insensitive
330+
if key not in used:
331+
used.add(key)
332+
return dest, candidate, (new_name if candidate != new_name else None)
333+
n += 1
334+
candidate = f"{stem}-{n}{suffix}"
335+
336+
337+
def validate_plan(plan, base_dir, expected_sources=None):
338+
"""Check a plan for defects that would lose or mis-file content.
339+
340+
Run this BEFORE execute_plan and refuse to proceed on any problem. A file
341+
missing from the plan is a file left unorganized, and a plan referencing a
342+
source that doesn't exist means the AI backend invented a filename -- both
343+
are worth catching before anything touches the filesystem.
344+
345+
Checks:
346+
- every `source` exists on disk
347+
- no `source` appears more than once
348+
- every expected source is present (when `expected_sources` is given)
349+
- duplicate category/new_name pairs are reported (informational -- these
350+
are auto-renamed by execute_plan, not fatal)
351+
352+
Args:
353+
plan: list of plan entry dicts
354+
base_dir: extraction directory the sources are relative to
355+
expected_sources: optional iterable of relpaths that MUST appear
356+
357+
Returns: list of human-readable problem strings ([] when the plan is clean)
358+
"""
359+
base_dir = Path(base_dir)
360+
problems = []
361+
362+
seen_sources = {}
363+
for i, entry in enumerate(plan):
364+
source = (entry.get("source") or "").replace("\\", "/")
365+
if not source:
366+
problems.append(f"entry {i}: missing 'source'")
367+
continue
368+
369+
seen_sources.setdefault(source, []).append(i)
370+
371+
if not (base_dir / source).exists():
372+
problems.append(
373+
f"entry {i}: source does not exist on disk: {source}"
374+
)
375+
376+
for source, idxs in sorted(seen_sources.items()):
377+
if len(idxs) > 1:
378+
problems.append(
379+
f"duplicate source: {source} appears {len(idxs)} times "
380+
f"(entries {idxs})"
381+
)
382+
383+
if expected_sources is not None:
384+
expected = {str(s).replace("\\", "/") for s in expected_sources}
385+
missing = sorted(expected - set(seen_sources))
386+
for m in missing:
387+
problems.append(f"missing from plan: {m}")
388+
389+
# Informational: execute_plan de-collides these, but a backend producing
390+
# many of them signals the prompt isn't being told what names exist yet.
391+
dests = {}
392+
for i, entry in enumerate(plan):
393+
key = (
394+
(entry.get("category") or "misc"),
395+
(entry.get("new_name") or ""),
396+
)
397+
if key[1]:
398+
dests.setdefault(key, []).append(i)
399+
for (cat, name), idxs in sorted(dests.items()):
400+
if len(idxs) > 1:
401+
problems.append(
402+
f"duplicate destination (will be auto-renamed): "
403+
f"{cat}/{name} claimed by entries {idxs}"
404+
)
405+
406+
return problems
407+
408+
307409
def execute_plan(plan, base_dir, linked_paths=None):
308410
"""
309411
Execute the organization plan by copying/renaming files.
@@ -324,7 +426,7 @@ def execute_plan(plan, base_dir, linked_paths=None):
324426
organized_dir = base_dir / "organized"
325427
organized_dir.mkdir(exist_ok=True)
326428

327-
stats = {"copied": 0, "linked": 0, "errors": 0}
429+
stats = {"copied": 0, "linked": 0, "errors": 0, "renamed": 0}
328430
categories = {}
329431
details = []
330432
organized_links = [] # Track which files were linked vs copied
@@ -333,6 +435,11 @@ def execute_plan(plan, base_dir, linked_paths=None):
333435
from .dedup import _can_create_symlink
334436
symlink_ok = _can_create_symlink()
335437

438+
# Destinations claimed during THIS run -- prevents one entry from silently
439+
# overwriting another when two entries propose the same category/new_name.
440+
used_dests = set()
441+
renames = []
442+
336443
for entry in plan:
337444
source = entry.get("source", "")
338445
category = entry.get("category", "misc")
@@ -353,7 +460,16 @@ def execute_plan(plan, base_dir, linked_paths=None):
353460
cat_dir = organized_dir / category
354461
cat_dir.mkdir(exist_ok=True)
355462

356-
dest_path = cat_dir / new_name
463+
# Claim a destination no other entry in this run is using. Renaming is
464+
# always reported -- a silent rename is nearly as bad as a silent
465+
# overwrite, because the summary would no longer describe the disk.
466+
dest_path, new_name, renamed_from = _resolve_unique_dest(
467+
cat_dir, new_name, used_dests)
468+
rename_note = ""
469+
if renamed_from:
470+
renames.append((source, category, renamed_from, new_name))
471+
stats["renamed"] += 1
472+
rename_note = f" [renamed from {renamed_from} -- name collision]"
357473

358474
# Check if this file is a dedup link
359475
source_resolved = source_path.resolve()
@@ -366,7 +482,8 @@ def execute_plan(plan, base_dir, linked_paths=None):
366482
if ok:
367483
stats["linked"] += 1
368484
categories.setdefault(category, []).append(new_name)
369-
details.append(f" {source} -> {category}/{new_name} [{link_type}]")
485+
details.append(
486+
f" {source} -> {category}/{new_name} [{link_type}]{rename_note}")
370487
organized_links.append({
371488
"rel_path": f"{category}/{new_name}",
372489
"canonical": str(canonical),
@@ -378,7 +495,8 @@ def execute_plan(plan, base_dir, linked_paths=None):
378495
shutil.copy2(str(source_path), str(dest_path))
379496
stats["copied"] += 1
380497
categories.setdefault(category, []).append(new_name)
381-
details.append(f" {source} -> {category}/{new_name} [copy fallback]")
498+
details.append(
499+
f" {source} -> {category}/{new_name} [copy fallback]{rename_note}")
382500
except Exception as e:
383501
stats["errors"] += 1
384502
details.append(f" ERROR copying {source}: {e}")
@@ -388,7 +506,7 @@ def execute_plan(plan, base_dir, linked_paths=None):
388506
shutil.copy2(str(source_path), str(dest_path))
389507
stats["copied"] += 1
390508
categories.setdefault(category, []).append(new_name)
391-
details.append(f" {source} -> {category}/{new_name}")
509+
details.append(f" {source} -> {category}/{new_name}{rename_note}")
392510
except Exception as e:
393511
stats["errors"] += 1
394512
details.append(f" ERROR copying {source}: {e}")
@@ -405,8 +523,19 @@ def execute_plan(plan, base_dir, linked_paths=None):
405523
summary_lines.append(f"- **Categories**: {len(categories)}")
406524
if stats["errors"]:
407525
summary_lines.append(f"- **Errors**: {stats['errors']}")
526+
if renames:
527+
summary_lines.append(f"- **Renamed (name collisions)**: {len(renames)}")
408528
summary_lines.append("")
409529

530+
if renames:
531+
summary_lines.append("## Renamed to avoid collisions\n")
532+
summary_lines.append(
533+
"Two or more files were assigned the same name. Each was kept as a "
534+
"separate file rather than overwritten:\n")
535+
for source, cat, was, now in renames:
536+
summary_lines.append(f"- `{source}`: `{cat}/{was}` -> `{cat}/{now}`")
537+
summary_lines.append("")
538+
410539
summary_lines.append("## Categories\n")
411540
for cat, files in sorted(categories.items()):
412541
summary_lines.append(f"### {cat} ({len(files)} files)")

notepad_cleanup/prompts/organize.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,8 @@ Return ONLY a JSON array. No markdown fencing, no explanation, just raw JSON:
3939
## Rules for naming and categorizing
4040

4141
- Every file MUST get its own `new_name` — never set it to null
42+
- **Every `category` + `new_name` pair MUST be unique across the whole plan.** Two files may share a category, and the same filename may appear in *different* categories, but no two entries may target the same `category/new_name`. If two files are genuinely similar, distinguish them by content (`grocery-list-june.txt` and `grocery-list-hardware-store.txt`), not by a numeric suffix
43+
- If you cannot tell two files apart well enough to name them distinctly, still give them different names — a duplicated name means one file would overwrite the other
4244
- Each tab is preserved as an individual file, even short notes
4345
- Use lowercase-with-dashes for folder and file names
4446
- Keep `.txt` for plain text and general notes

0 commit comments

Comments
 (0)