diff --git a/CLAUDE.md b/CLAUDE.md index ee7edc5..01c82d8 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -118,7 +118,7 @@ External knowledge sources are configured with active `*.inputs.toml` manifests - **Content-type classification**: Messages classified as `text`/`code`/`tool`/`reasoning` based on message content types during sync. Tool content is indexed in separate `search_items` rows with `content_type='tool'`. Pi agent reasoning blocks are captured when `index_reasoning=true` (default off) in the input manifest and indexed with `content_type='reasoning'`. Sync writes only to `search_items`; the `session_events` table was dropped in migration v5. - **Split FTS by retrieval semantics**: tool content (`content_type='tool'`) lives in a separate FTS5 index `tool_fts` (tokenizer `trigram`, substring/exact match for paths/commands/errors); prose content (text, code, reasoning) lives in `messages_fts` (`porter unicode61`). Migration v4 branched the triggers by content type. Migration v7 updated the triggers to route 'reasoning' alongside 'text'/'code' to `messages_fts`. `--content-type tool` queries `tool_fts`; prose queries `messages_fts`; an unfiltered query merges both via Reciprocal Rank Fusion (RRF, k=60), which fuses by rank position, not score magnitude, and is immune to incomparable cross-tokenizer BM25 scales. The trigram tokenizer matches substrings of ≥3 characters, so tool queries shorter than 3 characters will match zero results. - **Pure Go SQLite**: `modernc.org/sqlite` — no CGO, trivially cross-compilable. -- **Whitespace-insensitive schema normalization (fix #52 part 1)**: `normalizeSQL()` collapses runs of whitespace outside SQL string literals, preserving whitespace inside single-quoted literals and handling `''` escapes correctly. This eliminates cosmetic DDL formatting (e.g., `ALTER TABLE ADD COLUMN` inline vs. hand-wrapped multi-line) as a source of signature mismatch. All 76 manifest signatures are regenerated via `RegenerateManifestJSON` under the new normalization (reproducible: load fixtures, compute signatures under current `normalizeSQL`, write back manifest.json). The fix prevents false `unsupported_lineage` rejections when published-release-migrated databases encounter the catalog. +- **Whitespace-insensitive schema normalization (fix #52 part 1, PR #56)**: `normalizeSQL()` handles SQL lexical structure: line comments (`--...`), block comments (`/* ... */` non-nesting), single-quoted literals (with `''` escapes), double-quoted identifiers (with `""` escapes), backtick and bracket identifiers. Defect #1 (PR #56): apostrophes inside comments no longer flip quote state. Defect #2 (PR #56): double-quoted identifiers preserve internal whitespace (`"my table"` ≠ `"my table"`). Defect #3 (PR #56): comment-internal whitespace is collapsed (e.g., `-- note double` → `-- note double`), preventing comment formatting from being load-bearing. CRLF line endings in comments are normalized to LF for deterministic signatures across line ending styles. Block comments always have a single space before `*/` to preserve structure. This eliminates cosmetic DDL formatting (e.g., `ALTER TABLE ADD COLUMN` inline vs. hand-wrapped multi-line, or comments with varying spacing) as a source of signature mismatch. All 76 manifest signatures are regenerated via `RegenerateManifestJSON` under the new normalization (reproducible: load fixtures, compute signatures under current `normalizeSQL`, write back manifest.json). The fix prevents false `unsupported_lineage` rejections when published-release-migrated databases encounter the catalog. - **Startup coordination**: `github.com/gofrs/flock` via `internal/startuplock` — persistent `.startup-sync.lock` sidecar with `0600` permissions, OS-owned advisory lock, local-host-only WAL snapshot coordination, and read-only followers that never delete the sidecar. - **Connection pragmas via `_pragma`**: `modernc.org/sqlite` honors DSN pragmas only in the `_pragma=name(value)` form; the mattn-style `_name=value` (e.g. `_busy_timeout=5000`, `_journal_mode=WAL`) is silently ignored, which had left the DB in rollback (delete) journal mode with a zero busy timeout — the root cause of `database is locked` (SQLITE_BUSY) errors. Both connections in `internal/storage/storage.go` set `_pragma=journal_mode(WAL)`, `_pragma=synchronous(NORMAL)`, and `_pragma=busy_timeout(5000)` (read-only sets only the busy timeout; journal mode is persisted in the file). Always use `_pragma=name(value)` for any new connection pragma here. - **Autoupdate**: `picokit/autoupdate` fetches and stages the latest GitHub release in the background; `run()` waits up to 10s after the command completes so short-lived commands don't kill the download before it finishes. Autoupdate is mandatory: there is no runtime opt-out. `newUpdater()` in `cmd/backscroll/main.go` is the single wiring point — it calls `autoupdate.New` with no `envDisable`, and both `run()` and the wiring test go through it, so re-adding an opt-out would fail the test. Dev builds are exempt by identity — a plain `go build` yields `version="dev"`, which picokit never fetches or applies — so validate against a dev build, not an env var. `scripts/eval.sh` builds its own dev binary for the same reason (a release binary would fetch+wait ~10s per invocation). diff --git a/internal/compat/schema.go b/internal/compat/schema.go index dc09d7b..4f47842 100644 --- a/internal/compat/schema.go +++ b/internal/compat/schema.go @@ -412,41 +412,211 @@ func isFTSShadowObject(name string, virtualTables map[string]bool) bool { } func normalizeSQL(sqlText string) string { - // Collapse runs of whitespace outside string literals, but preserve whitespace - // within single-quoted SQL strings (including '' escapes). + // Collapse runs of whitespace outside SQL lexical constructs (comments, string literals, identifiers), + // preserving whitespace within: + // - Single-quoted string literals (including '' escapes) + // - Double-quoted identifiers (including "" escapes) + // - Backtick-quoted identifiers + // - Bracket-quoted identifiers [...] + // - Line comments (-- until end-of-line) + // - Block comments (/* ... */ non-nesting) + // + // Defect fix #1: Apostrophes and quotes within comments do not affect lexer state. + // Defect fix #2: Double-quoted identifiers preserve inner whitespace (e.g., "my table" != "my table"). + var result strings.Builder - inQuote := false lastWasSpace := false for i := 0; i < len(sqlText); i++ { ch := sqlText[i] - // Check for single quote (start or end of string literal, or '' escape) - if ch == '\'' { - // Check if this is a '' escape (two consecutive single quotes) - if inQuote && i+1 < len(sqlText) && sqlText[i+1] == '\'' { - // Write both quotes and skip the next one - result.WriteByte(ch) - result.WriteByte(ch) - i++ // skip the next quote + // Handle line comments (-- until end-of-line) + if ch == '-' && i+1 < len(sqlText) && sqlText[i+1] == '-' { + // Write the comment delimiter + result.WriteByte(ch) + i++ + result.WriteByte(sqlText[i]) + lastWasSpace = false + i++ + // Collect comment content until EOL, collapsing internal whitespace + var commentBuf strings.Builder + commentLastWasSpace := false + for i < len(sqlText) && sqlText[i] != '\n' { + if sqlText[i] == '\r' { + // Skip CR; will be normalized by outer whitespace logic + i++ + continue + } + if sqlText[i] == ' ' || sqlText[i] == '\t' { + if !commentLastWasSpace { + commentBuf.WriteByte(' ') + commentLastWasSpace = true + } + i++ + } else { + commentBuf.WriteByte(sqlText[i]) + commentLastWasSpace = false + i++ + } + } + // Write comment content, trimmed of trailing whitespace + commentContent := strings.TrimRight(commentBuf.String(), " \t") + result.WriteString(commentContent) + // If we found a newline, write it as a space (for collapsing purposes) + if i < len(sqlText) && sqlText[i] == '\n' { + result.WriteByte(' ') + lastWasSpace = true + i++ + } + i-- // Adjust for the outer loop's i++ + continue + } + + // Handle block comments (/* ... */ non-nesting) + if ch == '/' && i+1 < len(sqlText) && sqlText[i+1] == '*' { + // Write the comment opener + result.WriteByte(ch) + i++ + result.WriteByte(sqlText[i]) // '*' + lastWasSpace = false + i++ + // Collect comment content until */, collapsing internal whitespace + var commentBuf strings.Builder + commentLastWasSpace := false + for i < len(sqlText) { + if sqlText[i] == '*' && i+1 < len(sqlText) && sqlText[i+1] == '/' { + // End of comment found + break + } + if sqlText[i] == ' ' || sqlText[i] == '\t' || sqlText[i] == '\n' || sqlText[i] == '\r' { + if !commentLastWasSpace { + commentBuf.WriteByte(' ') + commentLastWasSpace = true + } + i++ + } else { + commentBuf.WriteByte(sqlText[i]) + commentLastWasSpace = false + i++ + } + } + // Write comment content, preserving structure (trim only leading/trailing multiples) + commentContent := strings.TrimSpace(commentBuf.String()) + if commentContent != "" { + result.WriteByte(' ') + result.WriteString(commentContent) + result.WriteByte(' ') + } else { + // Empty comment + result.WriteByte(' ') + } + // Write comment closer if found + if i < len(sqlText) && sqlText[i] == '*' && i+1 < len(sqlText) && sqlText[i+1] == '/' { + result.WriteByte('*') + i++ + result.WriteByte('/') + i++ lastWasSpace = false - continue } - // Toggle quote state - inQuote = !inQuote + i-- // Adjust for the outer loop's i++ + continue + } + + // Handle single-quoted string literals (preserve whitespace inside) + if ch == '\'' { + result.WriteByte(ch) + lastWasSpace = false + i++ + // Copy everything until closing single quote, handling '' escape + for i < len(sqlText) { + if sqlText[i] == '\'' { + result.WriteByte('\'') + // Check if this is an escape (followed by another single quote) + if i+1 < len(sqlText) && sqlText[i+1] == '\'' { + result.WriteByte('\'') + i += 2 + } else { + // End of string literal + i++ + lastWasSpace = false + break + } + } else { + result.WriteByte(sqlText[i]) + i++ + } + } + i-- // Adjust for the outer loop's i++ + continue + } + + // Handle double-quoted identifiers (preserve whitespace inside) + if ch == '"' { result.WriteByte(ch) lastWasSpace = false + i++ + // Copy everything until closing double quote, handling "" escape + for i < len(sqlText) { + if sqlText[i] == '"' { + result.WriteByte('"') + // Check if this is an escape (followed by another double quote) + if i+1 < len(sqlText) && sqlText[i+1] == '"' { + result.WriteByte('"') + i += 2 + } else { + // End of identifier + i++ + lastWasSpace = false + break + } + } else { + result.WriteByte(sqlText[i]) + i++ + } + } + i-- // Adjust for the outer loop's i++ continue } - // If inside quotes, preserve character as-is - if inQuote { + // Handle backtick-quoted identifiers (preserve whitespace inside) + if ch == '`' { result.WriteByte(ch) lastWasSpace = false + i++ + // Copy everything until closing backtick + for i < len(sqlText) && sqlText[i] != '`' { + result.WriteByte(sqlText[i]) + i++ + } + if i < len(sqlText) && sqlText[i] == '`' { + result.WriteByte('`') + i++ + lastWasSpace = false + } + i-- // Adjust for the outer loop's i++ + continue + } + + // Handle bracket-quoted identifiers [...] (preserve whitespace inside) + if ch == '[' { + result.WriteByte(ch) + lastWasSpace = false + i++ + // Copy everything until closing bracket + for i < len(sqlText) && sqlText[i] != ']' { + result.WriteByte(sqlText[i]) + i++ + } + if i < len(sqlText) && sqlText[i] == ']' { + result.WriteByte(']') + i++ + lastWasSpace = false + } + i-- // Adjust for the outer loop's i++ continue } - // Outside quotes: collapse whitespace + // Outside all special contexts: collapse whitespace if ch == ' ' || ch == '\t' || ch == '\n' || ch == '\r' { if !lastWasSpace { result.WriteByte(' ') diff --git a/internal/compat/schema_test.go b/internal/compat/schema_test.go index 9cac007..93edc8d 100644 --- a/internal/compat/schema_test.go +++ b/internal/compat/schema_test.go @@ -444,3 +444,210 @@ func TestNormalizeSQLPreservesWhitespaceInStringLiterals(t *testing.T) { t.Fatalf("whitespace in second literal not preserved: %q", norm) } } + +func TestNormalizeSQLHandlesApostropheInLineComment(t *testing.T) { + // Defect 1: apostrophe in line comment should not flip parser into quote mode + tests := []struct { + name string + sql string + want string + }{ + { + name: "apostrophe in line comment", + sql: "CREATE TABLE t ( -- don't\n a INTEGER,\n b TEXT\n)", + want: "CREATE TABLE t ( -- don't a INTEGER, b TEXT )", + }, + { + name: "apostrophe in line comment with real literal", + sql: "CREATE TABLE t ( -- don't do this\n body TEXT DEFAULT 'x y'\n)", + want: "CREATE TABLE t ( -- don't do this body TEXT DEFAULT 'x y' )", + }, + { + name: "the row's id in comment", + sql: "CREATE TABLE t ( a INTEGER -- the row's id\n)", + want: "CREATE TABLE t ( a INTEGER -- the row's id )", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + norm := normalizeSQL(tt.sql) + if norm != tt.want { + t.Fatalf("normalizeSQL(%q) =\n %q\nwant\n %q", tt.sql, norm, tt.want) + } + }) + } +} + +func TestNormalizeSQLHandlesApostropheInBlockComment(t *testing.T) { + // Apostrophe in block comment should not flip parser into quote mode + tests := []struct { + name string + sql string + want string + }{ + { + name: "apostrophe in block comment", + sql: "CREATE TABLE t ( /* don't */ a INTEGER )", + want: "CREATE TABLE t ( /* don't */ a INTEGER )", + }, + { + name: "quote in block comment", + sql: "CREATE TABLE t ( /* use 'single quotes' inside */ a INTEGER )", + want: "CREATE TABLE t ( /* use 'single quotes' inside */ a INTEGER )", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + norm := normalizeSQL(tt.sql) + if norm != tt.want { + t.Fatalf("normalizeSQL(%q) =\n %q\nwant\n %q", tt.sql, norm, tt.want) + } + }) + } +} + +func TestNormalizeSQLPreservesWhitespaceInDoubleQuotedIdentifiers(t *testing.T) { + // Defect 2: double-quoted identifiers must preserve inner whitespace + tests := []struct { + name string + sql string + want string + }{ + { + name: "double-quoted identifier with multiple spaces", + sql: `CREATE TABLE "my table" ( a INTEGER )`, + want: `CREATE TABLE "my table" ( a INTEGER )`, + }, + { + name: "double-quoted column name with spaces", + sql: `CREATE TABLE t ( "col name" INTEGER )`, + want: `CREATE TABLE t ( "col name" INTEGER )`, + }, + { + name: "double-quote escape in identifier", + sql: `CREATE TABLE "my""table" ( a INTEGER )`, + want: `CREATE TABLE "my""table" ( a INTEGER )`, + }, + { + name: "both single and double quoted identifiers", + sql: `CREATE TABLE "my table" ( id INTEGER, body TEXT DEFAULT 'x y' )`, + want: `CREATE TABLE "my table" ( id INTEGER, body TEXT DEFAULT 'x y' )`, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + norm := normalizeSQL(tt.sql) + if norm != tt.want { + t.Fatalf("normalizeSQL(%q) =\n %q\nwant\n %q", tt.sql, norm, tt.want) + } + }) + } +} + +func TestNormalizeSQLAdversarialCases(t *testing.T) { + // Various edge cases and adversarial inputs + tests := []struct { + name string + sql string + want string + }{ + { + name: "unterminated single quote", + sql: "CREATE TABLE t ( a TEXT DEFAULT 'x )", + want: "CREATE TABLE t ( a TEXT DEFAULT 'x )", + }, + { + name: "unterminated block comment", + sql: "CREATE TABLE t ( a INTEGER /* comment", + want: "CREATE TABLE t ( a INTEGER /* comment", + }, + { + name: "adjacent string literals", + sql: "CREATE TABLE t ( a TEXT DEFAULT 'x' 'y' )", + want: "CREATE TABLE t ( a TEXT DEFAULT 'x' 'y' )", + }, + { + name: "literal containing -- (line comment marker)", + sql: "CREATE TABLE t ( a TEXT DEFAULT 'foo -- bar' )", + want: "CREATE TABLE t ( a TEXT DEFAULT 'foo -- bar' )", + }, + { + name: "literal containing /* and */ (block comment markers)", + sql: "CREATE TABLE t ( a TEXT DEFAULT 'foo /* bar */ baz' )", + want: "CREATE TABLE t ( a TEXT DEFAULT 'foo /* bar */ baz' )", + }, + { + name: "empty string literal", + sql: "CREATE TABLE t ( a TEXT DEFAULT '' )", + want: "CREATE TABLE t ( a TEXT DEFAULT '' )", + }, + { + name: "comment containing -- marker", + sql: "CREATE TABLE t ( a INTEGER -- the -- marker )", + want: "CREATE TABLE t ( a INTEGER -- the -- marker )", + }, + { + name: "CRLF after line comment", + sql: "CREATE TABLE t ( a INTEGER -- comment\r\nb INTEGER )", + want: "CREATE TABLE t ( a INTEGER -- comment b INTEGER )", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + norm := normalizeSQL(tt.sql) + if norm != tt.want { + t.Fatalf("normalizeSQL(%q) =\n %q\nwant\n %q", tt.sql, norm, tt.want) + } + }) + } +} + +func TestNormalizeSQLCollapsesWhitespaceInsideComments(t *testing.T) { + // Comments are cosmetic — internal whitespace differences should not create + // different signatures. Two CREATE statements differing only in comment + // formatting should normalize identically (fix for latent defect #3). + tests := []struct { + name string + sql string + want string + }{ + { + name: "line comment with double space normalizes to single space", + sql: "CREATE TABLE t ( -- note double\na INTEGER )", + want: "CREATE TABLE t ( -- note double a INTEGER )", + }, + { + name: "line comment with multiple spaces", + sql: "CREATE TABLE t ( -- spaces everywhere \na INTEGER )", + want: "CREATE TABLE t ( -- spaces everywhere a INTEGER )", + }, + { + name: "block comment with double space normalizes to single space", + sql: "CREATE TABLE t ( /* note double */ a INTEGER )", + want: "CREATE TABLE t ( /* note double */ a INTEGER )", + }, + { + name: "block comment with mixed whitespace", + sql: "CREATE TABLE t ( /* multi space comment */ a INTEGER )", + want: "CREATE TABLE t ( /* multi space comment */ a INTEGER )", + }, + { + name: "inline comment inside CREATE with extra spaces", + sql: "CREATE TABLE t (\n -- inline comment\n a INTEGER\n)", + want: "CREATE TABLE t ( -- inline comment a INTEGER )", + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + norm := normalizeSQL(tt.sql) + if norm != tt.want { + t.Fatalf("normalizeSQL(%q) =\n %q\nwant\n %q", tt.sql, norm, tt.want) + } + }) + } +}