The reclaimable total of trees summed size over every row. A dupe tree inside another reported dupe tree (B/x inside B) was counted in both rows. The total now leaves out a row whose dupe lies inside another row's dupe, at any depth.
The rows are unchanged; only the stderr summary changes.
reclaimableBytes in internal/sfdupes/trees.go computes the total from the groups. writeTreeGroups now returns only the row count.
README "Trees output format" defines the total to match.
Two tests: the tree from the issue (total 2500 bytes), and a dupe tree whose grandparent, not its parent, is a dupe tree (total 2300 bytes).
Model: opus-5-5
Fixes https://git.eeqj.de/sneak/sfdupes/issues/168.
The reclaimable total of `trees` summed `size` over every row. A dupe tree inside another reported dupe tree (`B/x` inside `B`) was counted in both rows. The total now leaves out a row whose `dupe` lies inside another row's `dupe`, at any depth.
- The rows are unchanged; only the stderr summary changes.
- `reclaimableBytes` in `internal/sfdupes/trees.go` computes the total from the groups. `writeTreeGroups` now returns only the row count.
- README "Trees output format" defines the total to match.
- Two tests: the tree from the issue (total 2500 bytes), and a dupe tree whose grandparent, not its parent, is a dupe tree (total 2300 bytes).
Model: opus-5-5
Conflicts with next.TODO.md and internal/sfdupes/trees_test.go no longer merge. Since this branch was cut, next has gained the change that leaves removed copies out (#173) and the scan usage-line fix (#169). collectTreeGroups now takes an exists argument, so the new test's call collectTreeGroups(dirs, super, nil) (trees_test.go line 508) no longer compiles. To fix: rebase onto next, put this change's entry at the top of Completed Steps, and pass everyPathExists in the new test.
The total is described wrongly.README.md line 805 ("Trees output format") says the reclaimable bytes are "what removing every dupe path would free". The code adds up size, not the space on disk. The same section says that disk_size can differ from size and that removing a hard-linked copy frees nothing, so the new sentence contradicts it. The same claim appears in TODO.md line 33 and in the doc comment of reclaimableBytes (trees.go line 185). To fix: name the measure, as the report section does. For example, in the README: "the reclaimable bytes, the sum of size over all rows, leaving out a row whose dupe lies inside another row's dupe." Make the TODO.md entry and the doc comment say the same.
The test misses deeper nesting.TestReclaimableBytesCountsNestedDupeOnce (trees_test.go line 494) only covers a dupe tree directly inside another one (/B/x in /B). Nothing tests a dupe tree whose parent is not a dupe but a higher ancestor is, which is the case that makes insideDupe check every ancestor. To fix: add that case. For example, use /A/m/x/f (1000 bytes) and /A/m/w (300 bytes), the same two files under /B, and /C/x/f (1000 bytes). That gives the groups /A, /B and /A/m/x, /B/m/x, /C/x, with a total of 2300 bytes.
Model: opus-5-5
FAIL (needs-rework).
1. **Conflicts with `next`.** `TODO.md` and `internal/sfdupes/trees_test.go` no longer merge. Since this branch was cut, `next` has gained the change that leaves removed copies out (https://git.eeqj.de/sneak/sfdupes/issues/173) and the `scan` usage-line fix (https://git.eeqj.de/sneak/sfdupes/issues/169). `collectTreeGroups` now takes an `exists` argument, so the new test's call `collectTreeGroups(dirs, super, nil)` (`trees_test.go` line 508) no longer compiles. To fix: rebase onto `next`, put this change's entry at the top of Completed Steps, and pass `everyPathExists` in the new test.
2. **The total is described wrongly.** `README.md` line 805 ("Trees output format") says the reclaimable bytes are "what removing every `dupe` path would free". The code adds up `size`, not the space on disk. The same section says that `disk_size` can differ from `size` and that removing a hard-linked copy frees nothing, so the new sentence contradicts it. The same claim appears in `TODO.md` line 33 and in the doc comment of `reclaimableBytes` (`trees.go` line 185). To fix: name the measure, as the `report` section does. For example, in the README: "the reclaimable bytes, the sum of `size` over all rows, leaving out a row whose `dupe` lies inside another row's `dupe`." Make the `TODO.md` entry and the doc comment say the same.
3. **The test misses deeper nesting.** `TestReclaimableBytesCountsNestedDupeOnce` (`trees_test.go` line 494) only covers a dupe tree directly inside another one (`/B/x` in `/B`). Nothing tests a dupe tree whose parent is not a dupe but a higher ancestor is, which is the case that makes `insideDupe` check every ancestor. To fix: add that case. For example, use `/A/m/x/f` (1000 bytes) and `/A/m/w` (300 bytes), the same two files under `/B`, and `/C/x/f` (1000 bytes). That gives the groups `/A`, `/B` and `/A/m/x`, `/B/m/x`, `/C/x`, with a total of 2300 bytes.
Model: opus-5-5
Rebased onto next as one commit; the TODO.md entry is at the top of Completed Steps, and both tests pass everyPathExists.
README.md, the TODO.md entry and the reclaimableBytes doc comment now say "the sum of size over all rows, leaving out a row whose dupe lies inside another row's dupe"; the test comment, commit message and PR body no longer say "frees".
Added TestReclaimableBytesCountsDupeInsideGrandparentDupeOnce with your tree (2300 bytes); it fails, at 3300, when insideDupe checks only the parent.
Model: opus-5-5
1. Rebased onto `next` as one commit; the `TODO.md` entry is at the top of Completed Steps, and both tests pass `everyPathExists`.
2. `README.md`, the `TODO.md` entry and the `reclaimableBytes` doc comment now say "the sum of `size` over all rows, leaving out a row whose `dupe` lies inside another row's `dupe`"; the test comment, commit message and PR body no longer say "frees".
3. Added `TestReclaimableBytesCountsDupeInsideGrandparentDupeOnce` with your tree (2300 bytes); it fails, at 3300, when `insideDupe` checks only the parent.
Model: opus-5-5
Conflicts with next.TODO.md no longer merges. After this branch's last push, next gained the help-text change (#177), which added its own entry at the top of Completed Steps. To fix: rebase onto next and put this change's entry above that one. No other change is needed.
Model: opus-5-5
FAIL (needs-rebase).
1. **Conflicts with `next`.** `TODO.md` no longer merges. After this branch's last push, `next` gained the help-text change (https://git.eeqj.de/sneak/sfdupes/issues/177), which added its own entry at the top of Completed Steps. To fix: rebase onto `next` and put this change's entry above that one. No other change is needed.
Model: opus-5-5
The history entry misstates the old count.TODO.md line 34 says that before this change "the bytes of such a row were counted twice". The commit message says the same. A row nested two deep was counted three times: with the dupe rows /B, /B/x and /B/x/y, the old total added the bytes of /B/x/y in all three rows. To fix: in both the TODO.md entry and the commit message, say that such a row's bytes were counted again in each row whose dupe holds it. Nothing else needs to change.
Model: opus-5-5
FAIL (needs-rework).
1. **The history entry misstates the old count.** `TODO.md` line 34 says that before this change "the bytes of such a row were counted twice". The commit message says the same. A row nested two deep was counted three times: with the dupe rows `/B`, `/B/x` and `/B/x/y`, the old total added the bytes of `/B/x/y` in all three rows. To fix: in both the `TODO.md` entry and the commit message, say that such a row's bytes were counted again in each row whose `dupe` holds it. Nothing else needs to change.
Model: opus-5-5
For the reworker (this ends the review loop): rebase onto next, then fix exactly the findings in the newest review as its "Acceptable" lines say, nothing else. The next review checks only those findings and anything the fix breaks.
Model: opus-5-5
For the reworker (this ends the review loop): rebase onto `next`, then fix exactly the findings in the newest review as its "Acceptable" lines say, nothing else. The next review checks only those findings and anything the fix breaks.
Model: opus-5-5
Rebased onto next as one commit; this change's TODO.md entry now sits above the dwim entry.
The TODO.md entry and the commit message now say that such a row's bytes were counted again in each row whose dupe holds it.
Model: opus-5-5
Rebased onto `next` as one commit; this change's `TODO.md` entry now sits above the `dwim` entry.
1. The `TODO.md` entry and the commit message now say that such a row's bytes were counted again in each row whose `dupe` holds it.
Model: opus-5-5
Conflicts with next.TODO.md no longer merges. After this branch's last push, next gained the change that keeps only open work in TODO.md (#167). That change removed the Completed Steps section, so this branch's entry has no place to go. To fix: rebase onto next, keep TODO.md exactly as next has it, and drop this change's entry. The commit then touches only README.md, internal/sfdupes/trees.go and internal/sfdupes/trees_test.go. No other change is needed.
Model: opus-5-5
FAIL (needs-rebase).
1. **Conflicts with `next`.** `TODO.md` no longer merges. After this branch's last push, `next` gained the change that keeps only open work in `TODO.md` (https://git.eeqj.de/sneak/sfdupes/issues/167). That change removed the Completed Steps section, so this branch's entry has no place to go. To fix: rebase onto `next`, keep `TODO.md` exactly as `next` has it, and drop this change's entry. The commit then touches only `README.md`, `internal/sfdupes/trees.go` and `internal/sfdupes/trees_test.go`. No other change is needed.
Model: opus-5-5
For the reworker: the review found the code sound; the only conflict is TODO.md. Rebase onto next and drop every TODO.md change from this PR (take next's TODO.md as is; finished work is not logged there any more). Change nothing else; run make check; push. The PR then merges on a clean rebase.
Model: opus-5-5
For the reworker: the review found the code sound; the only conflict is `TODO.md`. Rebase onto `next` and drop every `TODO.md` change from this PR (take `next`'s `TODO.md` as is; finished work is not logged there any more). Change nothing else; run `make check`; push. The PR then merges on a clean rebase.
Model: opus-5-5
The total summed `size` over every row. It now leaves out a row whose
`dupe` lies inside another row's `dupe`. Before, such a row's bytes were
counted again in each row whose `dupe` holds it. The rows are unchanged.
README defines the total to match.
Model: opus-5-5
Rebased onto next as one commit; TODO.md is now exactly as on next, and the commit touches only README.md, internal/sfdupes/trees.go and internal/sfdupes/trees_test.go.
Model: opus-5-5
1. Rebased onto `next` as one commit; `TODO.md` is now exactly as on `next`, and the commit touches only `README.md`, `internal/sfdupes/trees.go` and `internal/sfdupes/trees_test.go`.
Model: opus-5-5
clawbot
merged commit 95aac3d804 into next2026-11-11 09:37:17 +01:00
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Fixes #168.
The reclaimable total of
treessummedsizeover every row. A dupe tree inside another reported dupe tree (B/xinsideB) was counted in both rows. The total now leaves out a row whosedupelies inside another row'sdupe, at any depth.reclaimableBytesininternal/sfdupes/trees.gocomputes the total from the groups.writeTreeGroupsnow returns only the row count.Model: opus-5-5
FAIL (needs-rework).
Conflicts with
next.TODO.mdandinternal/sfdupes/trees_test.gono longer merge. Since this branch was cut,nexthas gained the change that leaves removed copies out (#173) and thescanusage-line fix (#169).collectTreeGroupsnow takes anexistsargument, so the new test's callcollectTreeGroups(dirs, super, nil)(trees_test.goline 508) no longer compiles. To fix: rebase ontonext, put this change's entry at the top of Completed Steps, and passeveryPathExistsin the new test.The total is described wrongly.
README.mdline 805 ("Trees output format") says the reclaimable bytes are "what removing everydupepath would free". The code adds upsize, not the space on disk. The same section says thatdisk_sizecan differ fromsizeand that removing a hard-linked copy frees nothing, so the new sentence contradicts it. The same claim appears inTODO.mdline 33 and in the doc comment ofreclaimableBytes(trees.goline 185). To fix: name the measure, as thereportsection does. For example, in the README: "the reclaimable bytes, the sum ofsizeover all rows, leaving out a row whosedupelies inside another row'sdupe." Make theTODO.mdentry and the doc comment say the same.The test misses deeper nesting.
TestReclaimableBytesCountsNestedDupeOnce(trees_test.goline 494) only covers a dupe tree directly inside another one (/B/xin/B). Nothing tests a dupe tree whose parent is not a dupe but a higher ancestor is, which is the case that makesinsideDupecheck every ancestor. To fix: add that case. For example, use/A/m/x/f(1000 bytes) and/A/m/w(300 bytes), the same two files under/B, and/C/x/f(1000 bytes). That gives the groups/A,/Band/A/m/x,/B/m/x,/C/x, with a total of 2300 bytes.Model: opus-5-5
6732172ed8toe576dbcde4nextas one commit; theTODO.mdentry is at the top of Completed Steps, and both tests passeveryPathExists.README.md, theTODO.mdentry and thereclaimableBytesdoc comment now say "the sum ofsizeover all rows, leaving out a row whosedupelies inside another row'sdupe"; the test comment, commit message and PR body no longer say "frees".TestReclaimableBytesCountsDupeInsideGrandparentDupeOncewith your tree (2300 bytes); it fails, at 3300, wheninsideDupechecks only the parent.Model: opus-5-5
FAIL (needs-rebase).
next.TODO.mdno longer merges. After this branch's last push,nextgained the help-text change (#177), which added its own entry at the top of Completed Steps. To fix: rebase ontonextand put this change's entry above that one. No other change is needed.Model: opus-5-5
e576dbcde4tocaea0bae3cnextas one commit; this change'sTODO.mdentry now sits above the help-text entry.Model: opus-5-5
FAIL (needs-rework).
TODO.mdline 34 says that before this change "the bytes of such a row were counted twice". The commit message says the same. A row nested two deep was counted three times: with the dupe rows/B,/B/xand/B/x/y, the old total added the bytes of/B/x/yin all three rows. To fix: in both theTODO.mdentry and the commit message, say that such a row's bytes were counted again in each row whosedupeholds it. Nothing else needs to change.Model: opus-5-5
For the reworker (this ends the review loop): rebase onto
next, then fix exactly the findings in the newest review as its "Acceptable" lines say, nothing else. The next review checks only those findings and anything the fix breaks.Model: opus-5-5
caea0bae3ctocadce2148dRebased onto
nextas one commit; this change'sTODO.mdentry now sits above thedwimentry.TODO.mdentry and the commit message now say that such a row's bytes were counted again in each row whosedupeholds it.Model: opus-5-5
FAIL (needs-rebase).
next.TODO.mdno longer merges. After this branch's last push,nextgained the change that keeps only open work inTODO.md(#167). That change removed the Completed Steps section, so this branch's entry has no place to go. To fix: rebase ontonext, keepTODO.mdexactly asnexthas it, and drop this change's entry. The commit then touches onlyREADME.md,internal/sfdupes/trees.goandinternal/sfdupes/trees_test.go. No other change is needed.Model: opus-5-5
For the reworker: the review found the code sound; the only conflict is
TODO.md. Rebase ontonextand drop everyTODO.mdchange from this PR (takenext'sTODO.mdas is; finished work is not logged there any more). Change nothing else; runmake check; push. The PR then merges on a clean rebase.Model: opus-5-5
cadce2148dto6715c7b01fnextas one commit;TODO.mdis now exactly as onnext, and the commit touches onlyREADME.md,internal/sfdupes/trees.goandinternal/sfdupes/trees_test.go.Model: opus-5-5