From c355ef4d25919ac55ca117276794d24e4170d0c5 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Mon, 21 Sep 2026 21:41:57 +0200 Subject: [PATCH] Report a prune count that could not be read as unknown, not 0 (closes #96) Prune read table row counts before and after to report how many orphaned files, chunks and blobs it removed, and discarded the error from every read. A failed query therefore reported as a count of 0, and the summary showed plausible wrong numbers. A count that cannot be read is now logged as a warning (on stderr, also under --json) and shown as "unknown"; a difference computed from an unknown count is itself unknown. 0 still means the table was empty. No --json document carries these counts, so none can show a false 0. model: claude-opus-4-8 (implementation, review); claude-fable-5-1 (merge) --- TODO.md | 11 ++- internal/vaultik/prune_count_test.go | 79 ++++++++++++++++++++ internal/vaultik/snapshot.go | 105 ++++++++++++++++++++------- 3 files changed, 168 insertions(+), 27 deletions(-) create mode 100644 internal/vaultik/prune_count_test.go diff --git a/TODO.md b/TODO.md index c61e0d2..28d4c28 100644 --- a/TODO.md +++ b/TODO.md @@ -25,6 +25,16 @@ release" is exactly the contradiction # Completed Steps +- 2026-09-21: Stopped `prune` from reporting a failed row count as 0 + ([issue #96](https://git.eeqj.de/sneak/vaultik/issues/96)). The seven + `getTableCount` reads in `PruneDatabase` discarded their error, so a + query that could not run became a plausible `0` and the before/after + delta computed from it looked like real work. Each read now logs at + warn on failure and renders as `unknown`, never `0`, so an empty table + is distinguishable from one that could not be queried. The counts have + no `--json` representation — under `--json` the summary is suppressed + entirely — so nothing there can show a false `0`. + - 2026-09-21: Made the s3 storage backend report a missing object as `storage.ErrNotFound`, like the `file` and `rclone` backends and as the `Storer` interface documents. `S3Storer.Get` and `Stat` returned the raw @@ -33,7 +43,6 @@ release" is exactly the contradiction helper (reused by `HeadObject`) and a test that a missing key maps to `ErrNotFound` ([issue #129](https://git.eeqj.de/sneak/vaultik/issues/129)). - - 2026-09-21: Fixed `verify --deep` reporting healthy snapshots as corrupt. Its final blob-integrity check hashed the encrypted downloaded bytes with a single SHA256 and compared that to the blob diff --git a/internal/vaultik/prune_count_test.go b/internal/vaultik/prune_count_test.go new file mode 100644 index 0000000..fbc3e5d --- /dev/null +++ b/internal/vaultik/prune_count_test.go @@ -0,0 +1,79 @@ +package vaultik //nolint:testpackage // exercises unexported count helpers + +import ( + "context" + "testing" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + "sneak.berlin/go/vaultik/internal/database" + "sneak.berlin/go/vaultik/internal/log" +) + +// TestTableCountForReportSurfacesReadFailure is the regression guard for +// the discarded-error bug: getTableCount for a table its query cannot +// resolve must not silently become 0. A count that could not be read is +// reported as unknown, which a reader can tell apart from an empty table. +// +//nolint:paralleltest // installs the global logger via log.Initialize +func TestTableCountForReportSurfacesReadFailure(t *testing.T) { + log.Initialize(log.Config{}) + + ctx := context.Background() + + db, err := database.New(ctx, ":memory:") + require.NoError(t, err) + t.Cleanup(func() { _ = db.Close() }) + + v := &Vaultik{DB: db} + v.SetContext(ctx) + + // A table present in the schema reads as a real count. + blobs := v.tableCountForReport("blobs") + require.NotNil(t, blobs, "an existing table must read as a real count") + assert.Equal(t, int64(0), *blobs) + + // A syntactically valid name the sanitizer accepts but whose table + // the query cannot resolve is the exact shape #96 describes: a + // would-be loud failure that used to be discarded into a 0. + _, err = v.getTableCount("snapshots_missing") + require.Error(t, err, "a query against a nonexistent table must fail") + + missing := v.tableCountForReport("snapshots_missing") + assert.Nil(t, missing, "a failed read is unknown, not a count") + + // The rendered count for a failed read must say unknown, never 0. + assert.Equal(t, countUnknown, countText(missing)) + assert.NotEqual(t, "0", countText(missing)) +} + +// TestCountTextDistinguishesEmptyFromUnknown pins the distinction the +// output has to preserve: 0 means the table was empty, "unknown" means +// the count could not be read. +func TestCountTextDistinguishesEmptyFromUnknown(t *testing.T) { + t.Parallel() + + zero := int64(0) + seven := int64(7) + + assert.Equal(t, "0", countText(&zero)) + assert.Equal(t, "7", countText(&seven)) + assert.Equal(t, countUnknown, countText(nil)) +} + +// TestCountDiffUnknownWhenEitherSideUnknown checks that a delta computed +// from an unreadable count is itself unknown rather than a plausible +// number. +func TestCountDiffUnknownWhenEitherSideUnknown(t *testing.T) { + t.Parallel() + + before := int64(10) + after := int64(3) + + require.NotNil(t, countDiff(&before, &after)) + assert.Equal(t, int64(7), *countDiff(&before, &after)) + + assert.Nil(t, countDiff(nil, &after), "unknown before yields unknown delta") + assert.Nil(t, countDiff(&before, nil), "unknown after yields unknown delta") + assert.Nil(t, countDiff(nil, nil)) +} diff --git a/internal/vaultik/snapshot.go b/internal/vaultik/snapshot.go index 16bba39..8c12ec7 100644 --- a/internal/vaultik/snapshot.go +++ b/internal/vaultik/snapshot.go @@ -8,6 +8,7 @@ import ( "path/filepath" "regexp" "sort" + "strconv" "strings" "time" @@ -1540,12 +1541,17 @@ func (v *Vaultik) outputRemoveJSON(result *RemoveResult) error { return encoder.Encode(result) } -// PruneResult contains statistics about the prune operation +// PruneResult contains statistics about the prune operation. +// SnapshotsDeleted counts snapshots actually deleted. FilesDeleted, +// ChunksDeleted, and BlobsDeleted are derived from before/after row +// counts of the local index; each is nil when a count could not be read, +// so an unreadable count is reported as unknown rather than silently +// as 0. type PruneResult struct { SnapshotsDeleted int64 - FilesDeleted int64 - ChunksDeleted int64 - BlobsDeleted int64 + FilesDeleted *int64 + ChunksDeleted *int64 + BlobsDeleted *int64 } // PruneDatabase removes incomplete snapshots and orphaned files, chunks, @@ -1560,7 +1566,7 @@ func (v *Vaultik) PruneDatabase() (*PruneResult, error) { result := &PruneResult{} // Snapshot counts before deletion of incompletes. - snapshotCountBefore, _ := v.getTableCount("snapshots") + snapshotCountBefore := v.tableCountForReport("snapshots") // First, delete any incomplete snapshots incompleteSnapshots, err := v.Repositories.Snapshots.GetIncompleteSnapshots(v.ctx) @@ -1575,9 +1581,9 @@ func (v *Vaultik) PruneDatabase() (*PruneResult, error) { } // Get counts before cleanup for reporting - fileCountBefore, _ := v.getTableCount("files") - chunkCountBefore, _ := v.getTableCount("chunks") - blobCountBefore, _ := v.getTableCount("blobs") + fileCountBefore := v.tableCountForReport("files") + chunkCountBefore := v.tableCountForReport("chunks") + blobCountBefore := v.tableCountForReport("blobs") // Run the cleanup err = v.SnapshotManager.CleanupOrphanedData(v.ctx) @@ -1586,36 +1592,83 @@ func (v *Vaultik) PruneDatabase() (*PruneResult, error) { } // Get counts after cleanup - fileCountAfter, _ := v.getTableCount("files") - chunkCountAfter, _ := v.getTableCount("chunks") - blobCountAfter, _ := v.getTableCount("blobs") + fileCountAfter := v.tableCountForReport("files") + chunkCountAfter := v.tableCountForReport("chunks") + blobCountAfter := v.tableCountForReport("blobs") - result.FilesDeleted = fileCountBefore - fileCountAfter - result.ChunksDeleted = chunkCountBefore - chunkCountAfter - result.BlobsDeleted = blobCountBefore - blobCountAfter + result.FilesDeleted = countDiff(fileCountBefore, fileCountAfter) + result.ChunksDeleted = countDiff(chunkCountBefore, chunkCountAfter) + result.BlobsDeleted = countDiff(blobCountBefore, blobCountAfter) log.Info("Local database prune complete", "incomplete_snapshots", result.SnapshotsDeleted, - "orphaned_files", result.FilesDeleted, - "orphaned_chunks", result.ChunksDeleted, - "orphaned_blobs", result.BlobsDeleted, + "orphaned_files", countText(result.FilesDeleted), + "orphaned_chunks", countText(result.ChunksDeleted), + "orphaned_blobs", countText(result.BlobsDeleted), ) - snapshotCountAfter := snapshotCountBefore - result.SnapshotsDeleted + // Snapshots remaining after removing the incomplete ones; unknown if + // the pre-prune snapshot count could not be read. + snapshotsRemain := countDiff(snapshotCountBefore, &result.SnapshotsDeleted) v.UI.Completef("Pruned local index database.") - v.UI.Detailf("Incomplete snapshots: %d removed (%d remain).", - result.SnapshotsDeleted, snapshotCountAfter) - v.UI.Detailf("Orphaned files: %d removed (%d remain).", - result.FilesDeleted, fileCountAfter) - v.UI.Detailf("Orphaned chunks: %d removed (%d remain).", - result.ChunksDeleted, chunkCountAfter) - v.UI.Detailf("Orphaned blobs: %d removed (%d remain).", - result.BlobsDeleted, blobCountAfter) + v.UI.Detailf("Incomplete snapshots: %s removed (%s remain).", + countText(&result.SnapshotsDeleted), countText(snapshotsRemain)) + v.UI.Detailf("Orphaned files: %s removed (%s remain).", + countText(result.FilesDeleted), countText(fileCountAfter)) + v.UI.Detailf("Orphaned chunks: %s removed (%s remain).", + countText(result.ChunksDeleted), countText(chunkCountAfter)) + v.UI.Detailf("Orphaned blobs: %s removed (%s remain).", + countText(result.BlobsDeleted), countText(blobCountAfter)) return result, nil } +// countUnknown is what a count reads as when its query could not be run, +// distinct from "0", which means the table really was empty. +const countUnknown = "unknown" + +// tableCountForReport returns the row count of a table for the prune +// summary, or nil if the count could not be read. A read failure is +// logged at warn — visible even under --json, which routes warnings to +// stderr — and then rendered as unknown rather than silently becoming 0, +// so a broken query is a visible failure instead of a plausible wrong +// number. +func (v *Vaultik) tableCountForReport(tableName string) *int64 { + count, err := v.getTableCount(tableName) + if err != nil { + log.Warn("could not read table row count for prune summary", + "table", tableName, "error", err) + + return nil + } + + return &count +} + +// countDiff returns before-after, or nil if either count is unknown so +// that an unreadable count does not collapse into a plausible delta. +func countDiff(before, after *int64) *int64 { + if before == nil || after == nil { + return nil + } + + diff := *before - *after + + return &diff +} + +// countText renders a count that may be unknown: nil (the read failed) +// becomes "unknown", never "0", so a reader can tell an empty table from +// one that could not be queried. +func countText(count *int64) string { + if count == nil { + return countUnknown + } + + return strconv.FormatInt(*count, 10) +} + // validTableNameRe matches table names containing only lowercase // alphanumeric characters and underscores. var validTableNameRe = regexp.MustCompile(`^[a-z0-9_]+$`)