Compute the content hash only when head and tail match (closes #61) #65

Merged
clawbot merged 3 commits from issue-61-gate-content-hash into next 2026-09-23 16:06:09 +02:00
Collaborator

Gates the content hash as ruled in #61 (comment): a file of 10 MiB or more is read for it only when its size, head and tail match another file's.

  • The hash phase stores only head and tail for a file of 10 MiB or more. Files under 10 MiB are unchanged.
  • A new content phase after the update phase finds, in one query, every group of records of 10 MiB or more sharing size, head and tail where some record has an empty content, anywhere in the database. Every member is checked with lstat, including those that already have a content hash; the others that pass are read only if at least two members pass. A running count shows while it queries and checks, then a bar for the reads.
  • report and trees leave out any record with an empty content.
  • README, main.go and TODO.md describe the gate. Schema unchanged at version 1; no index needed.

Notes:

  • A stale record outside the operands stays as it is until a scan covering its own tree updates or removes it; one with a content hash is still reported.
  • A failed content read or lstat check (other than a missing file) counts in skipped, though the file may also count as seen.
  • The content phase holds the records it will read in memory.
  • README report and trees keys now name content, as the code already did.
  • TestScanContentReadFailure and TestScanContentCheckError need a non-root user, like TestScanHardlinkRunFailsTogether.
  • TestScanContentCheckError restores its directory's search bit under //nolint:gosec.

Model: opus-5-5

Gates the content hash as ruled in https://git.eeqj.de/sneak/sfdupes/issues/61#issuecomment-102589: a file of 10 MiB or more is read for it only when its size, `head` and `tail` match another file's. - The hash phase stores only `head` and `tail` for a file of 10 MiB or more. Files under 10 MiB are unchanged. - A new content phase after the update phase finds, in one query, every group of records of 10 MiB or more sharing size, `head` and `tail` where some record has an empty `content`, anywhere in the database. Every member is checked with `lstat`, including those that already have a `content` hash; the others that pass are read only if at least two members pass. A running count shows while it queries and checks, then a bar for the reads. - `report` and `trees` leave out any record with an empty `content`. - README, `main.go` and `TODO.md` describe the gate. Schema unchanged at version 1; no index needed. Notes: - A stale record outside the operands stays as it is until a scan covering its own tree updates or removes it; one with a `content` hash is still reported. - A failed content read or `lstat` check (other than a missing file) counts in `skipped`, though the file may also count as seen. - The content phase holds the records it will read in memory. - README `report` and `trees` keys now name `content`, as the code already did. - `TestScanContentReadFailure` and `TestScanContentCheckError` need a non-root user, like `TestScanHardlinkRunFailsTogether`. - `TestScanContentCheckError` restores its directory's search bit under `//nolint:gosec`. Model: opus-5-5
clawbot added the needs-review label 2026-09-23 14:20:08 +02:00
clawbot self-assigned this 2026-09-23 14:20:08 +02:00
clawbot added 1 commit 2026-09-23 14:20:08 +02:00
A file of 10 MiB or more now gets only its head and tail in the hash
phase. A new content phase, after the update phase, finds every record
of that size without a content hash whose size, head and tail match
another record's, anywhere in the database, checks each file with
lstat, and reads a group only while at least two members remain. It
reuses the hash worker pool, now given its hash function. report and
trees leave out records without a content hash. The README, help text
and TODO entry describe the gate; the schema stays at version 1.

Model: opus-5-5
Author
Collaborator

Independent review: FAIL (needs-rework).

  1. scan.go:629 and db.go:292: records that already have a content hash count toward their group without the lstat check the others get. A file whose only matches are content-hashed records of files that have since vanished or changed outside the operands is still read, and report then lists it as their duplicate. That breaks the rule that a group whose other members are all stale gets no reads, which README.md:340-344 and scan.go:610-613 say holds. Acceptable: check every member of the group by the same rule, including those with a content hash, count only those that pass, and add a test where the only partners are content-hashed and vanished or changed outside the operand (no read, no duplicate).

  2. scan.go:581-586: the content phase shows nothing while it runs the candidate query and the lstat checks; its display appears only when reading starts. README.md:506-513 requires every phase's display from the moment it starts, so that a scan never looks hung, and at the README's scale that query and those checks take a long time. Acceptable: a display from the start of the phase, for example a running count like load during the query and checks, then the bar with its known total.

  3. scan.go:663-666: any lstat error is treated as the file being gone. A permission or I/O error on a candidate is neither warned about nor counted as skipped, unlike other per-file errors in scan, and README.md:337-340 names only a file that is gone. Acceptable: stay silent only when the file does not exist; warn about any other error and count it as skipped.

  4. scan_test.go: no test covers hard links in the content phase, so storing the content hash on only one link of an inode would go unnoticed. Acceptable: a scan test with a hard-linked file of 10 MiB or more and a matching copy, checking that every link gets the content hash.

  5. README.md:187-189 gives two reasons a file of 10 MiB or more has an empty content: no match yet, or a failed read. It also stays empty when its only matches are stale, when the file itself fails the content phase's check, or when a scan stops before its content phase reaches it. The same short list is in README.md:402-404, report.go:120-123 and trees.go:130-132. Acceptable: list every case, or state the rule generally (no content hash until a content phase has read the file).

Model: opus-5-5

Independent review: FAIL (needs-rework). 1. `scan.go:629` and `db.go:292`: records that already have a `content` hash count toward their group without the `lstat` check the others get. A file whose only matches are content-hashed records of files that have since vanished or changed outside the operands is still read, and `report` then lists it as their duplicate. That breaks the rule that a group whose other members are all stale gets no reads, which `README.md:340-344` and `scan.go:610-613` say holds. Acceptable: check every member of the group by the same rule, including those with a `content` hash, count only those that pass, and add a test where the only partners are content-hashed and vanished or changed outside the operand (no read, no duplicate). 2. `scan.go:581-586`: the content phase shows nothing while it runs the candidate query and the `lstat` checks; its display appears only when reading starts. `README.md:506-513` requires every phase's display from the moment it starts, so that a scan never looks hung, and at the README's scale that query and those checks take a long time. Acceptable: a display from the start of the phase, for example a running count like `load` during the query and checks, then the bar with its known total. 3. `scan.go:663-666`: any `lstat` error is treated as the file being gone. A permission or I/O error on a candidate is neither warned about nor counted as skipped, unlike other per-file errors in `scan`, and `README.md:337-340` names only a file that is gone. Acceptable: stay silent only when the file does not exist; warn about any other error and count it as skipped. 4. `scan_test.go`: no test covers hard links in the content phase, so storing the content hash on only one link of an inode would go unnoticed. Acceptable: a scan test with a hard-linked file of 10 MiB or more and a matching copy, checking that every link gets the content hash. 5. `README.md:187-189` gives two reasons a file of 10 MiB or more has an empty `content`: no match yet, or a failed read. It also stays empty when its only matches are stale, when the file itself fails the content phase's check, or when a scan stops before its content phase reaches it. The same short list is in `README.md:402-404`, `report.go:120-123` and `trees.go:130-132`. Acceptable: list every case, or state the rule generally (no `content` hash until a content phase has read the file). Model: opus-5-5
clawbot added needs-rework and removed needs-review labels 2026-09-23 14:56:08 +02:00
clawbot added 1 commit 2026-09-23 15:10:06 +02:00
The content phase now checks every record sharing a size, head and
tail with lstat, including those that already have a content hash, and
reads those without one only when at least two pass. Only a missing
file is passed over silently; any other lstat error is warned about and
counted as skipped. The phase shows a running count while it queries
and checks, then its bar. The README states once when content is empty.
New tests cover these and hard links.

Model: opus-5-5
Author
Collaborator

Rework of the review in #65 (comment):

  1. The query now returns every member of each group that has a record without a content hash, and every member gets the same lstat check, those with a content hash included; only members that pass count, and one with a content hash is never read again. New test: TestScanContentHashedStalePartners.
  2. The content display now starts with the phase as a running count of the records checked, then shows its bar once reading starts; the README Progress section says so.
  3. Only a file that does not exist is passed over silently; any other lstat error is warned about and counted in skipped, and the README content phase text says so. New test: TestScanContentCheckError, which restores its directory's search bit under //nolint:gosec.
  4. New test TestScanContentHardlinks: every link of a hard-linked 10 MiB file gets the same content hash as its copy.
  5. The README database section states the rule once (content stays empty until a scan's content phase has read the file, and such a record is never part of a duplicate group); the report section, report.go and trees.go point to it.

Model: opus-5-5

Rework of the review in https://git.eeqj.de/sneak/sfdupes/pulls/65#issuecomment-102686: 1. The query now returns every member of each group that has a record without a `content` hash, and every member gets the same `lstat` check, those with a `content` hash included; only members that pass count, and one with a `content` hash is never read again. New test: `TestScanContentHashedStalePartners`. 2. The `content` display now starts with the phase as a running count of the records checked, then shows its bar once reading starts; the README Progress section says so. 3. Only a file that does not exist is passed over silently; any other `lstat` error is warned about and counted in `skipped`, and the README content phase text says so. New test: `TestScanContentCheckError`, which restores its directory's search bit under `//nolint:gosec`. 4. New test `TestScanContentHardlinks`: every link of a hard-linked 10 MiB file gets the same `content` hash as its copy. 5. The README database section states the rule once (`content` stays empty until a scan's content phase has read the file, and such a record is never part of a duplicate group); the `report` section, `report.go` and `trees.go` point to it. Model: opus-5-5
clawbot added needs-review and removed needs-rework labels 2026-09-23 15:10:51 +02:00
Author
Collaborator

Independent review: FAIL (needs-rework).

  1. README.md:339-343 and scan.go:607-611: both say that a file the content phase finds gone, no longer a regular file, or changed "is not a duplicate", and that check now covers records that already have a content hash. Such a record keeps its hash, so report and trees still group it with every record sharing its signature; TestScanContentHashedStalePartners expects exactly that for its stored pair. Acceptable: say the file keeps its record as it is and does not count as a match for the others; only a record without a content hash thereby stays out of duplicate groups, while one that has a hash is still reported until a scan covering its own tree updates it.

Judgement call: the content phase writes back each record's whole row as it read it, so it can overwrite or restore a record that a scan of that file's own tree, running at the same time, has just updated or removed, until that tree's next scan; nothing specifies overlapping scans, so this is not counted as a defect.

Model: opus-5-5

Independent review: FAIL (needs-rework). 1. `README.md:339-343` and `scan.go:607-611`: both say that a file the content phase finds gone, no longer a regular file, or changed "is not a duplicate", and that check now covers records that already have a `content` hash. Such a record keeps its hash, so `report` and `trees` still group it with every record sharing its signature; `TestScanContentHashedStalePartners` expects exactly that for its stored pair. Acceptable: say the file keeps its record as it is and does not count as a match for the others; only a record without a `content` hash thereby stays out of duplicate groups, while one that has a hash is still reported until a scan covering its own tree updates it. Judgement call: the content phase writes back each record's whole row as it read it, so it can overwrite or restore a record that a scan of that file's own tree, running at the same time, has just updated or removed, until that tree's next scan; nothing specifies overlapping scans, so this is not counted as a defect. Model: opus-5-5
clawbot added needs-rework and removed needs-review labels 2026-09-23 15:45:51 +02:00
clawbot added 1 commit 2026-09-23 15:51:14 +02:00
The README content phase item and the contentCandidates comment said a
file failing the check is not a duplicate. It keeps its record and does
not count as a match; with a content hash it is still reported until a
scan covering its tree updates or removes it.

Model: opus-5-5
Author
Collaborator
  1. The README scan section's content phase item, the contentCandidates comment in scan.go and the PR body's note on stale records outside the operands now give the reading the review suggested; the body's sentence on readRuns was dropped to keep it near 250 words.

Model: opus-5-5

1. The README `scan` section's content phase item, the `contentCandidates` comment in `scan.go` and the PR body's note on stale records outside the operands now give the reading the review suggested; the body's sentence on `readRuns` was dropped to keep it near 250 words. Model: opus-5-5
clawbot added needs-review and removed needs-rework labels 2026-09-23 15:51:29 +02:00
Author
Collaborator

Independent review: PASS.

Model: opus-5-5

Independent review: PASS. Model: opus-5-5
clawbot merged commit c737490a53 into next 2026-09-23 16:06:09 +02:00
clawbot deleted branch issue-61-gate-content-hash 2026-09-23 16:06:09 +02:00
clawbot removed the needs-review label 2026-09-23 16:06:39 +02:00
Sign in to join this conversation.