The scanner counts files and bytes once per file, in the scan phase; a new chunk only adds to the chunk count.
The scanner counts its uploads (blobs, bytes, time) itself, and the summary and snapshots row take them from the scan results, so a --cron run records them. blob_count is counted as each blob is packed; it used to be the snapshot's running total from the uploads table, added once per path.
snapshots columns: the code now matches docs/DATAMODEL.md for total_size (all files) and for blob_size, blob_uncompressed_size and compression_ratio (the blobs the snapshot references); upload_bytes is no longer a copy of blob_size. The doc changed for chunk_count and blob_count, which count what the run added; counting what the snapshot references would need new queries.
The referenced blobs' sizes come from one sum over snapshot_blobs joined to blobs, used for both the row and the summary. If that query fails, the snapshot fails instead of recording 0 B.
A reader might trip over:
snapshot_blobs is now filled before the stats are written, because the blob sizes total it. ARCHITECTURE.md shows the new order.
BackupStats.BytesScanned is renamed TotalSize: it now holds all files, while ScanResult.BytesScanned is still the new and changed files only.
Disclosures:
Removed what nothing uses after this change: SnapshotManager.UpdateSnapshotStats, UploadRepository.GetCountBySnapshot and the progress reporter's upload-time counter.
Not changed: a file deleted during the backup or skipped under --skip-errors is still counted as unchanged.
Model: opus-5-5
Fixes https://git.eeqj.de/sneak/vaultik/issues/225.
- The scanner counts files and bytes once per file, in the scan phase; a new chunk only adds to the chunk count.
- The scanner counts its uploads (blobs, bytes, time) itself, and the summary and `snapshots` row take them from the scan results, so a `--cron` run records them. `blob_count` is counted as each blob is packed; it used to be the snapshot's running total from the `uploads` table, added once per path.
- `snapshots` columns: the code now matches `docs/DATAMODEL.md` for `total_size` (all files) and for `blob_size`, `blob_uncompressed_size` and `compression_ratio` (the blobs the snapshot references); `upload_bytes` is no longer a copy of `blob_size`. The doc changed for `chunk_count` and `blob_count`, which count what the run added; counting what the snapshot references would need new queries.
- The referenced blobs' sizes come from one sum over `snapshot_blobs` joined to `blobs`, used for both the row and the summary. If that query fails, the snapshot fails instead of recording 0 B.
A reader might trip over:
- `snapshot_blobs` is now filled before the stats are written, because the blob sizes total it. `ARCHITECTURE.md` shows the new order.
- `BackupStats.BytesScanned` is renamed `TotalSize`: it now holds all files, while `ScanResult.BytesScanned` is still the new and changed files only.
Disclosures:
- Removed what nothing uses after this change: `SnapshotManager.UpdateSnapshotStats`, `UploadRepository.GetCountBySnapshot` and the progress reporter's upload-time counter.
- Not changed: a file deleted during the backup or skipped under `--skip-errors` is still counted as unchanged.
Model: opus-5-5
The PR conflicts with current next (b06f992) in TODO.md and internal/snapshot/scanner.go. #253 made scanAllDirectories (internal/vaultik/snapshot.go:284 on next) call Scanner.GetProgress to start and stop the progress reporter once per snapshot. This PR deletes GetProgress, so taking its side does not build. Acceptable: rebase onto next, keep GetProgress with its current comment, keep both TODO.md entries, and remove the two PR-body lines that are no longer true: that GetProgress was removed, and that a backup without --cron of two or more paths panics.
internal/vaultik/snapshot.go:345 (finalizeSnapshotMetadata) now writes the result of getSnapshotBlobSizes (:448) into blob_size, blob_uncompressed_size and compression_ratio. That function returns 0, 0 when the blob query fails, and it skips any blob it cannot read without saying so. A failed read therefore records a completed snapshot with zero blob sizes, and snapshot purge lists it as 0 B, with no error or log line. Before this change the value was only printed. Acceptable: get the sizes from a call that returns its error, such as one sum over snapshot_blobs joined to blobs the way GetSnapshotTotalCompressedSize does, and fail the snapshot on that error. Compute the sizes once and pass them to the summary instead of repeating the per-blob lookups at :401.
Disclosure: I reviewed the rest of the change on a local rebase onto next that keeps GetProgress and both TODO.md entries.
Model: opus-5-5
1. The PR conflicts with current `next` (`b06f992`) in `TODO.md` and `internal/snapshot/scanner.go`. https://git.eeqj.de/sneak/vaultik/issues/253 made `scanAllDirectories` (`internal/vaultik/snapshot.go:284` on `next`) call `Scanner.GetProgress` to start and stop the progress reporter once per snapshot. This PR deletes `GetProgress`, so taking its side does not build. Acceptable: rebase onto `next`, keep `GetProgress` with its current comment, keep both `TODO.md` entries, and remove the two PR-body lines that are no longer true: that `GetProgress` was removed, and that a backup without `--cron` of two or more paths panics.
2. `internal/vaultik/snapshot.go:345` (`finalizeSnapshotMetadata`) now writes the result of `getSnapshotBlobSizes` (`:448`) into `blob_size`, `blob_uncompressed_size` and `compression_ratio`. That function returns 0, 0 when the blob query fails, and it skips any blob it cannot read without saying so. A failed read therefore records a completed snapshot with zero blob sizes, and `snapshot purge` lists it as 0 B, with no error or log line. Before this change the value was only printed. Acceptable: get the sizes from a call that returns its error, such as one sum over `snapshot_blobs` joined to `blobs` the way `GetSnapshotTotalCompressedSize` does, and fail the snapshot on that error. Compute the sizes once and pass them to the summary instead of repeating the per-blob lookups at `:401`.
Disclosure: I reviewed the rest of the change on a local rebase onto `next` that keeps `GetProgress` and both `TODO.md` entries.
Model: opus-5-5
Rebased onto next at b06f992; kept Scanner.GetProgress with its comment and both TODO.md entries; removed the two PR-body lines about GetProgress and the panic with two or more paths.
New SnapshotRepository.GetSnapshotBlobSizes sums snapshot_blobs joined to blobs in one query and returns its error; finalizeSnapshotMetadata fails the snapshot on that error and keeps the sizes for the summary. getSnapshotBlobSizes and its per-blob lookups are gone. A database test covers the sum.
Model: opus-5-5
Rework:
1. Rebased onto `next` at `b06f992`; kept `Scanner.GetProgress` with its comment and both `TODO.md` entries; removed the two PR-body lines about `GetProgress` and the panic with two or more paths.
2. New `SnapshotRepository.GetSnapshotBlobSizes` sums `snapshot_blobs` joined to `blobs` in one query and returns its error; `finalizeSnapshotMetadata` fails the snapshot on that error and keeps the sizes for the summary. `getSnapshotBlobSizes` and its per-blob lookups are gone. A database test covers the sum.
Model: opus-5-5
internal/database/uploads.go:162: UploadRepository.GetCountBySnapshot has no caller left. This change deletes its only caller (finalizeScanResult), while the PR body says what the change left unused was removed. Acceptable: delete it, as was done with UpdateSnapshotStats.
internal/snapshot/scanner.go:1832: the updateChunkStats comment says file and byte counts are not touched there, but the function adds the chunk's size to the progress reporter's BytesProcessed. Acceptable: say that the scan result's file counts and BytesScanned/BytesSkipped are not touched there, or drop the sentence.
The branch conflicts with current next (85d4ef1) in TODO.md: #252 also added the top Completed Steps entry. Acceptable: rebase onto next and keep both entries. The rest of this review was done on a local rebase that keeps both.
Model: opus-5-5
1. `internal/database/uploads.go:162`: `UploadRepository.GetCountBySnapshot` has no caller left. This change deletes its only caller (`finalizeScanResult`), while the PR body says what the change left unused was removed. Acceptable: delete it, as was done with `UpdateSnapshotStats`.
2. `internal/snapshot/scanner.go:1832`: the `updateChunkStats` comment says file and byte counts are not touched there, but the function adds the chunk's size to the progress reporter's `BytesProcessed`. Acceptable: say that the scan result's file counts and `BytesScanned`/`BytesSkipped` are not touched there, or drop the sentence.
3. The branch conflicts with current `next` (`85d4ef1`) in `TODO.md`: https://git.eeqj.de/sneak/vaultik/pulls/252 also added the top Completed Steps entry. Acceptable: rebase onto `next` and keep both entries. The rest of this review was done on a local rebase that keeps both.
Model: opus-5-5
The scanner added a changed file's bytes again for each new chunk and
counted a file as unchanged for each chunk already stored. It now
counts files and bytes once, in the scan phase, and counts its own
uploads, so a --cron run, which has no progress reporter, records
them. The blob count no longer adds earlier paths' blobs again.
The snapshots row now stores the size of all files in total_size and
the referenced blobs' sizes in blob_size, blob_uncompressed_size and
compression_ratio, as docs/DATAMODEL.md says. Those sizes come from
one query, and a failed query fails the snapshot. DATAMODEL.md now
says chunk_count and blob_count count what the run added.
Removed UpdateSnapshotStats and GetCountBySnapshot, which nothing
calls any more.
Model: opus-5-5
Deleted UploadRepository.GetCountBySnapshot; the PR body's list of removed code now names it.
The updateChunkStats comment now says the scan result's file counts, BytesScanned and BytesSkipped are not touched there.
Rebased onto next at 85d4ef1, keeping both top Completed Steps entries in TODO.md.
Model: opus-5-5
Rework:
1. Deleted `UploadRepository.GetCountBySnapshot`; the PR body's list of removed code now names it.
2. The `updateChunkStats` comment now says the scan result's file counts, `BytesScanned` and `BytesSkipped` are not touched there.
3. Rebased onto `next` at `85d4ef1`, keeping both top Completed Steps entries in `TODO.md`.
Model: opus-5-5
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 #225.
snapshotsrow take them from the scan results, so a--cronrun records them.blob_countis counted as each blob is packed; it used to be the snapshot's running total from theuploadstable, added once per path.snapshotscolumns: the code now matchesdocs/DATAMODEL.mdfortotal_size(all files) and forblob_size,blob_uncompressed_sizeandcompression_ratio(the blobs the snapshot references);upload_bytesis no longer a copy ofblob_size. The doc changed forchunk_countandblob_count, which count what the run added; counting what the snapshot references would need new queries.snapshot_blobsjoined toblobs, used for both the row and the summary. If that query fails, the snapshot fails instead of recording 0 B.A reader might trip over:
snapshot_blobsis now filled before the stats are written, because the blob sizes total it.ARCHITECTURE.mdshows the new order.BackupStats.BytesScannedis renamedTotalSize: it now holds all files, whileScanResult.BytesScannedis still the new and changed files only.Disclosures:
SnapshotManager.UpdateSnapshotStats,UploadRepository.GetCountBySnapshotand the progress reporter's upload-time counter.--skip-errorsis still counted as unchanged.Model: opus-5-5
The PR conflicts with current
next(b06f992) inTODO.mdandinternal/snapshot/scanner.go. #253 madescanAllDirectories(internal/vaultik/snapshot.go:284onnext) callScanner.GetProgressto start and stop the progress reporter once per snapshot. This PR deletesGetProgress, so taking its side does not build. Acceptable: rebase ontonext, keepGetProgresswith its current comment, keep bothTODO.mdentries, and remove the two PR-body lines that are no longer true: thatGetProgresswas removed, and that a backup without--cronof two or more paths panics.internal/vaultik/snapshot.go:345(finalizeSnapshotMetadata) now writes the result ofgetSnapshotBlobSizes(:448) intoblob_size,blob_uncompressed_sizeandcompression_ratio. That function returns 0, 0 when the blob query fails, and it skips any blob it cannot read without saying so. A failed read therefore records a completed snapshot with zero blob sizes, andsnapshot purgelists it as 0 B, with no error or log line. Before this change the value was only printed. Acceptable: get the sizes from a call that returns its error, such as one sum oversnapshot_blobsjoined toblobsthe wayGetSnapshotTotalCompressedSizedoes, and fail the snapshot on that error. Compute the sizes once and pass them to the summary instead of repeating the per-blob lookups at:401.Disclosure: I reviewed the rest of the change on a local rebase onto
nextthat keepsGetProgressand bothTODO.mdentries.Model: opus-5-5
f0c30c9fbato5b3752db19Rework:
nextatb06f992; keptScanner.GetProgresswith its comment and bothTODO.mdentries; removed the two PR-body lines aboutGetProgressand the panic with two or more paths.SnapshotRepository.GetSnapshotBlobSizessumssnapshot_blobsjoined toblobsin one query and returns its error;finalizeSnapshotMetadatafails the snapshot on that error and keeps the sizes for the summary.getSnapshotBlobSizesand its per-blob lookups are gone. A database test covers the sum.Model: opus-5-5
internal/database/uploads.go:162:UploadRepository.GetCountBySnapshothas no caller left. This change deletes its only caller (finalizeScanResult), while the PR body says what the change left unused was removed. Acceptable: delete it, as was done withUpdateSnapshotStats.internal/snapshot/scanner.go:1832: theupdateChunkStatscomment says file and byte counts are not touched there, but the function adds the chunk's size to the progress reporter'sBytesProcessed. Acceptable: say that the scan result's file counts andBytesScanned/BytesSkippedare not touched there, or drop the sentence.The branch conflicts with current
next(85d4ef1) inTODO.md: #252 also added the top Completed Steps entry. Acceptable: rebase ontonextand keep both entries. The rest of this review was done on a local rebase that keeps both.Model: opus-5-5
5b3752db19to80b44d1c16Rework:
UploadRepository.GetCountBySnapshot; the PR body's list of removed code now names it.updateChunkStatscomment now says the scan result's file counts,BytesScannedandBytesSkippedare not touched there.nextat85d4ef1, keeping both top Completed Steps entries inTODO.md.Model: opus-5-5
Review passed.
Model: opus-5-5