Remove all ctime usage and storage #55

clawbot · 2026-03-19T14:08:26+01:00

clawbot commented

2026-03-19 14:08:26 +01:00

Remove all ctime from the codebase per sneak's decision on PR #48.

Rationale

ctime means different things on macOS (birth time) vs Linux (inode change time) — ambiguous cross-platform
Vaultik never uses ctime operationally (scanning triggers on mtime change)
Cannot be restored on either platform
Write-only forensic data with no consumer

Changes

Schema (internal/database/schema.sql): Removed ctime column from files table
Model (internal/database/models.go): Removed CTime field from File struct
Database layer (internal/database/files.go): Removed ctime from all INSERT/SELECT queries, ON CONFLICT updates, and scan targets in both scanFile and scanFileRows helpers; updated CreateBatch accordingly
Scanner (internal/snapshot/scanner.go): Removed CTime: info.ModTime() assignment in checkFileInMemory()
Tests: Removed all CTime field assignments from 8 test files
Documentation: Removed ctime references from ARCHITECTURE.md and docs/DATAMODEL.md

docker build . passes clean (lint, fmt-check, all tests).

closes #54

Remove all ctime from the codebase per sneak's decision on [PR #48](https://git.eeqj.de/sneak/vaultik/pulls/48). ## Rationale - ctime means different things on macOS (birth time) vs Linux (inode change time) — ambiguous cross-platform - Vaultik never uses ctime operationally (scanning triggers on mtime change) - Cannot be restored on either platform - Write-only forensic data with no consumer ## Changes - **Schema** (`internal/database/schema.sql`): Removed `ctime` column from `files` table - **Model** (`internal/database/models.go`): Removed `CTime` field from `File` struct - **Database layer** (`internal/database/files.go`): Removed ctime from all INSERT/SELECT queries, ON CONFLICT updates, and scan targets in both `scanFile` and `scanFileRows` helpers; updated `CreateBatch` accordingly - **Scanner** (`internal/snapshot/scanner.go`): Removed `CTime: info.ModTime()` assignment in `checkFileInMemory()` - **Tests**: Removed all `CTime` field assignments from 8 test files - **Documentation**: Removed ctime references from `ARCHITECTURE.md` and `docs/DATAMODEL.md` `docker build .` passes clean (lint, fmt-check, all tests). closes #54

clawbot added 1 commit 2026-03-19 14:08:26 +01:00

remove ctime column from schema, model, queries, scanner, and docs

check / check (pull_request) Successful in 4m19s

Details

1717677288

ctime is ambiguous cross-platform (macOS birth time vs Linux inode change
time), never used operationally (scanning triggers on mtime), cannot be
restored on either platform, and was write-only forensic data with no
consumer.

Removes ctime from:
- files table schema (schema.sql)
- File struct (models.go)
- all SQL queries and scan targets (files.go)
- scanner file metadata collection (scanner.go)
- all test files
- ARCHITECTURE.md and docs/DATAMODEL.md

closes #54

clawbot added the needs-review label 2026-03-19 14:09:01 +01:00

clawbot reviewed 2026-03-19 14:11:09 +01:00

clawbot left a comment

Review: Remove all ctime usage and storage

ctime removal: ✅ PASS

The ctime removal itself is thorough and correct:

schema.sql: ctime column removed from files table ✅
models.go: CTime field removed from File struct ✅
files.go: All INSERT/SELECT/ON CONFLICT/scan targets updated — Create, GetByPath, GetByID, GetByPathTx, scanFile, scanFileRows, ListModifiedSince, ListByPrefix, ListAll, CreateBatch all clean ✅
scanner.go: CTime: info.ModTime() assignment removed from checkFileInMemory() ✅
Tests: All 8 test files updated, no CTime in any struct literal ✅
Docs: ARCHITECTURE.md and docs/DATAMODEL.md updated ✅
Full codebase grep: zero remaining matches for ctime or CTime (case-insensitive) ✅
docker build .: passes clean (lint, fmt-check, all tests, build) ✅
Batch comment updated: 10 values → 9 values, placeholder counts adjusted ✅

Out-of-scope change: ❌ NEEDS FIX

The PR also silently removes ON DELETE CASCADE from two foreign keys unrelated to ctime:

-- snapshot_files: was
FOREIGN KEY (file_id) REFERENCES files(id) ON DELETE CASCADE
-- now
FOREIGN KEY (file_id) REFERENCES files(id)

-- snapshot_blobs: was
FOREIGN KEY (blob_id) REFERENCES blobs(id) ON DELETE CASCADE
-- now
FOREIGN KEY (blob_id) REFERENCES blobs(id)

This is a behavioral change that affects data cleanup. Previously, deleting a file from files would cascade-delete its snapshot_files junction rows. Now it won't — orphaned junction rows will accumulate unless the application's orphan cleanup handles this explicitly.

This change:

Is not mentioned in the PR description
Is unrelated to ctime removal
Could silently break orphan cleanup if the app relies on cascade behavior anywhere

Please revert these two FK changes. If removing cascades is intentional, it deserves its own PR (issue #54 is specifically about ctime) with proper justification and verification that orphan cleanup still works correctly without cascades.

Verdict: REQUEST_CHANGES — the ctime work is perfect, but the unrelated FK cascade removal must be reverted or split out.

## Review: Remove all ctime usage and storage ### ctime removal: ✅ PASS The ctime removal itself is thorough and correct: - **schema.sql**: `ctime` column removed from `files` table ✅ - **models.go**: `CTime` field removed from `File` struct ✅ - **files.go**: All INSERT/SELECT/ON CONFLICT/scan targets updated — `Create`, `GetByPath`, `GetByID`, `GetByPathTx`, `scanFile`, `scanFileRows`, `ListModifiedSince`, `ListByPrefix`, `ListAll`, `CreateBatch` all clean ✅ - **scanner.go**: `CTime: info.ModTime()` assignment removed from `checkFileInMemory()` ✅ - **Tests**: All 8 test files updated, no `CTime` in any struct literal ✅ - **Docs**: `ARCHITECTURE.md` and `docs/DATAMODEL.md` updated ✅ - **Full codebase grep**: zero remaining matches for `ctime` or `CTime` (case-insensitive) ✅ - **`docker build .`**: passes clean (lint, fmt-check, all tests, build) ✅ - **Batch comment updated**: `10 values` → `9 values`, placeholder counts adjusted ✅ ### Out-of-scope change: ❌ NEEDS FIX The PR also silently removes `ON DELETE CASCADE` from two foreign keys unrelated to ctime: ```sql -- snapshot_files: was FOREIGN KEY (file_id) REFERENCES files(id) ON DELETE CASCADE -- now FOREIGN KEY (file_id) REFERENCES files(id) -- snapshot_blobs: was FOREIGN KEY (blob_id) REFERENCES blobs(id) ON DELETE CASCADE -- now FOREIGN KEY (blob_id) REFERENCES blobs(id) ``` This is a **behavioral change** that affects data cleanup. Previously, deleting a file from `files` would cascade-delete its `snapshot_files` junction rows. Now it won't — orphaned junction rows will accumulate unless the application's orphan cleanup handles this explicitly. This change: 1. Is not mentioned in the PR description 2. Is unrelated to ctime removal 3. Could silently break orphan cleanup if the app relies on cascade behavior anywhere **Please revert these two FK changes.** If removing cascades is intentional, it deserves its own PR ([issue #54](https://git.eeqj.de/sneak/vaultik/issues/54) is specifically about ctime) with proper justification and verification that orphan cleanup still works correctly without cascades. **Verdict: REQUEST_CHANGES** — the ctime work is perfect, but the unrelated FK cascade removal must be reverted or split out.

clawbot added needs-rework and removed needs-review labels 2026-03-19 14:12:12 +01:00

clawbot added 1 commit 2026-03-19 14:15:43 +01:00

fix: restore ON DELETE CASCADE on snapshot_files.file_id and snapshot_blobs.blob_id FKs

check / check (pull_request) Successful in 2m26s

Details

15f0172e0c

clawbot commented

2026-03-19 14:15:52 +01:00

Rework complete — restored ON DELETE CASCADE on two foreign keys in internal/database/schema.sql that were accidentally dropped during the ctime removal:

snapshot_files.file_id FK → REFERENCES files(id) ON DELETE CASCADE
snapshot_blobs.blob_id FK → REFERENCES blobs(id) ON DELETE CASCADE

No other changes. docker build . passes clean (lint, fmt-check, all tests, build).

**Rework complete** — restored `ON DELETE CASCADE` on two foreign keys in `internal/database/schema.sql` that were accidentally dropped during the ctime removal: - `snapshot_files.file_id` FK → `REFERENCES files(id) ON DELETE CASCADE` - `snapshot_blobs.blob_id` FK → `REFERENCES blobs(id) ON DELETE CASCADE` No other changes. `docker build .` passes clean (lint, fmt-check, all tests, build).

clawbot added needs-review and removed needs-rework labels 2026-03-19 14:16:16 +01:00

clawbot reviewed 2026-03-19 14:17:40 +01:00

clawbot left a comment

Review: PR #55 — Remove all ctime usage and storage (post-rework)

ctime removal: ✅ PASS

schema.sql: ctime column removed from files table ✅
models.go: CTime field removed from File struct ✅
files.go: All INSERT/SELECT/ON CONFLICT/scan targets updated — Create, GetByPath, GetByID, GetByPathTx, scanFile, scanFileRows, ListModifiedSince, ListByPrefix, ListAll, CreateBatch all clean ✅
scanner.go: CTime: info.ModTime() assignment removed from checkFileInMemory() ✅
Tests: All 8 test files updated, zero CTime references remain ✅
Docs: ARCHITECTURE.md and docs/DATAMODEL.md updated ✅
Full codebase grep: grep -riIn 'ctime' --include='*.go' --include='*.sql' --include='*.md' — zero matches ✅

ON DELETE CASCADE fix: ✅ PASS

Previous review found two FKs had silently lost ON DELETE CASCADE:

snapshot_files.file_id → REFERENCES files(id) ON DELETE CASCADE ✅ restored
snapshot_blobs.blob_id → REFERENCES blobs(id) ON DELETE CASCADE ✅ restored

All 12 FK declarations in schema.sql verified — cascade behavior matches the original schema (cascades on entity ID references, no cascade on content-addressed chunk_hash/blob_hash references).

Build: ✅ PASS

docker build . passes clean (lint, fmt-check, all tests, build).

Scope: ✅ CLEAN

15 files changed, all scoped to ctime removal + FK restoration. No unrelated changes.

Verdict: PASS — clean, correct, well-scoped. Ready for merge.

## Review: [PR #55](https://git.eeqj.de/sneak/vaultik/pulls/55) — Remove all ctime usage and storage (post-rework) ### ctime removal: ✅ PASS - **schema.sql**: `ctime` column removed from `files` table ✅ - **models.go**: `CTime` field removed from `File` struct ✅ - **files.go**: All INSERT/SELECT/ON CONFLICT/scan targets updated — `Create`, `GetByPath`, `GetByID`, `GetByPathTx`, `scanFile`, `scanFileRows`, `ListModifiedSince`, `ListByPrefix`, `ListAll`, `CreateBatch` all clean ✅ - **scanner.go**: `CTime: info.ModTime()` assignment removed from `checkFileInMemory()` ✅ - **Tests**: All 8 test files updated, zero `CTime` references remain ✅ - **Docs**: `ARCHITECTURE.md` and `docs/DATAMODEL.md` updated ✅ - **Full codebase grep**: `grep -riIn 'ctime' --include='*.go' --include='*.sql' --include='*.md'` — zero matches ✅ ### ON DELETE CASCADE fix: ✅ PASS Previous review found two FKs had silently lost `ON DELETE CASCADE`: - `snapshot_files.file_id` → `REFERENCES files(id) ON DELETE CASCADE` ✅ restored - `snapshot_blobs.blob_id` → `REFERENCES blobs(id) ON DELETE CASCADE` ✅ restored All 12 FK declarations in `schema.sql` verified — cascade behavior matches the original schema (cascades on entity ID references, no cascade on content-addressed chunk_hash/blob_hash references). ### Build: ✅ PASS `docker build .` passes clean (lint, fmt-check, all tests, build). ### Scope: ✅ CLEAN 15 files changed, all scoped to ctime removal + FK restoration. No unrelated changes. **Verdict: PASS** — clean, correct, well-scoped. Ready for merge.

clawbot added merge-ready and removed needs-review labels 2026-03-19 14:18:13 +01:00

sneak was assigned by clawbot

2026-03-19 14:18:13 +01:00

sneak merged commit 1c72a37bc8 into main

2026-03-20 03:12:47 +01:00

sneak deleted branch remove-ctime

2026-03-20 03:12:47 +01:00

sneak referenced this issue from a commit

2026-03-20 03:12:49 +01:00

Remove all ctime usage and storage (#55)

clawbot referenced this pull request

2026-03-20 06:55:54 +01:00