Skip to content

Commit 1fe9359

Browse files
committed
Various correctness, robustness, and performance fixes
Validation and routing: - Add word-boundary checks to command validation so DESCRIBING/SELECTS/ INSERTION etc. no longer match DESCRIBE/SELECT/INSERT prefixes - Rewrite stripComments in router and batch/utils to respect quoted strings (was corrupting URLs and SQL with -- inside quotes) - Use db.ConvertToJSONQuery for SELECT->SELECT JSON transform so SELECT DISTINCT is handled correctly UDT decoder: - Treat empty wire data for text/varchar/ascii as empty string and empty blob as []byte{}, instead of nil. Cassandra signals null with a -1 length prefix, not zero-length payload - Drop defensive overflow checks on smallint/int/bigint that are impossible by two's-complement decoding; annotate with nolint:gosec - Fix DATE encoding test: Cassandra DATE is days_since_epoch + 2^31, not raw int32. Add pre-epoch test case Config robustness: - Add OutputFormat field so default format can be set in config - Surface cqlshrc permission/parse errors via LoadWarnings instead of silent swallowing - Resolve relative credentials = path against the cqlshrc dir, not CWD - Mask password/api/secret keys in debug logs; stop logging raw credentials-file lines Session manager: - SetKeyspace and SetOutputFormat now return error - Validate keyspace name against ^[a-zA-Z_][a-zA-Z0-9_]*$ - Read default OutputFormat from config DB session/executor: - Read pageSize from config (was hardcoded 100) - Remove bogus close-iterator-then-recreate dance from ExecuteSelectQuery that always ran the query twice Batch: - Quote-aware splitter and comment stripper (handles ''-escapes and BATCH boundaries) - Precompute column-name index map in CSV output (O(rows*cols) instead of O(rows*cols^2)) AI: - Validate extracted JSON with json.Valid; add extractBalancedJSON to match nested objects instead of LastIndex("}") - Bound conversation cache to 100 entries with 24h TTL eviction - Debounce search-index "last accessed" updates (every 30s, batched) instead of one goroutine per match - Switch ai_handler to NewAIWithCache Parquet/COPY: - Sort partition column names so output is deterministic across runs - Track partition file size from disk after Flush for accurate maxFileSize accounting - Fix COPY header-row leak: skip Data[0] which contains headers - Narrow UUID heuristic to "name contains uuid" only; "id" was a false positive for columns like provider_id, valid UI: - sliding_window MaxRows=0 now means unlimited (memory limit only) - Log SetKeyspace error after the new error return Meta-commands: - PAGING OFF now sets pageSize to 0 (server defaults) instead of 10000, in both code paths Add AGENTS.md repository contributor guide Add tests for batch/utils, db/session, and validation/command
1 parent b0c96e8 commit 1fe9359

26 files changed

Lines changed: 1028 additions & 289 deletions

AGENTS.md

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
# Repository Guidelines
2+
3+
## Project Structure & Module Organization
4+
- `cmd/cqlai/main.go` is the CLI entry point for the `cqlai` binary.
5+
- `internal/` holds core application packages (parsing, routing, UI, storage, etc.).
6+
- `assets/` contains branding and UI assets used by docs and release artifacts.
7+
- `docs/` includes user guides and feature documentation (Parquet, AI config, batch mode).
8+
- `test/` hosts test suites; `test/parquet/` for Parquet I/O unit tests and `test/integration/` for Cassandra-backed integration tests.
9+
- `scripts/` and `bin/` hold helper scripts and build outputs.
10+
- `cqlai.json.example` shows configuration defaults.
11+
12+
## Build, Test, and Development Commands
13+
- `make build`: build the binary into `bin/`.
14+
- `make run`: build and launch locally.
15+
- `make test`: run all Go tests with race detection and coverage output.
16+
- `make test-coverage`: generate `coverage.html` from `coverage.out`.
17+
- `make lint`: run `golangci-lint` if available, otherwise `go vet`.
18+
- `make fmt`: format with `go fmt` and `goimports` (if installed).
19+
- `make grammar`: regenerate ANTLR grammar sources in `internal/parser/grammar/`.
20+
21+
## Coding Style & Naming Conventions
22+
- Go standard formatting is required; use `make fmt` before pushing.
23+
- Follow idiomatic Go naming (mixedCaps for exported identifiers, lowerCamel for locals).
24+
- Keep package names short and lowercase; avoid underscores unless required by tools.
25+
26+
## Testing Guidelines
27+
- Unit tests use Go’s `testing` package and live alongside code as `*_test.go`.
28+
- Parquet unit tests: `go test -v ./test/parquet/...` or `./test/parquet/run_tests.sh`.
29+
- Integration tests require a running Cassandra instance; see `test/integration/README.md`.
30+
- No explicit coverage threshold is documented; aim to cover new logic.
31+
32+
## Commit & Pull Request Guidelines
33+
- Commit messages follow a conventional style seen in history (`feat:`, `fix:`, `perf:`, `chore:`).
34+
- PRs should include a clear description, linked issues when applicable, and test notes.
35+
- For UI or output changes, add terminal screenshots or recordings when it improves review clarity.
36+
37+
## Configuration & Security
38+
- Local config is read from `cqlai.json` or `~/.cqlai.json`.
39+
- Do not commit API keys or secrets; use `cqlai.json.example` as a template.

internal/ai/client.go

Lines changed: 55 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -168,24 +168,73 @@ type Message struct {
168168

169169
// extractJSON attempts to extract JSON from a text response
170170
func extractJSON(text string) string {
171-
// Look for JSON between ```json and ``` markers
171+
// First priority: Look for JSON between ```json and ``` markers
172172
startMarker := JSONStartMarker
173173
endMarker := JSONEndMarker
174174
startIdx := strings.Index(text, startMarker)
175175
if startIdx != -1 {
176176
startIdx += len(startMarker)
177177
endIdx := strings.Index(text[startIdx:], endMarker)
178178
if endIdx != -1 {
179-
return strings.TrimSpace(text[startIdx : startIdx+endIdx])
179+
candidate := strings.TrimSpace(text[startIdx : startIdx+endIdx])
180+
// Validate it's actual JSON
181+
if json.Valid([]byte(candidate)) {
182+
return candidate
183+
}
180184
}
181185
}
182186

183-
// Look for JSON between { and }
187+
// Second priority: Extract balanced JSON object
184188
startIdx = strings.Index(text, "{")
185189
if startIdx != -1 {
186-
endIdx := strings.LastIndex(text, "}")
187-
if endIdx != -1 && endIdx > startIdx {
188-
return text[startIdx : endIdx+1]
190+
// Find the matching closing brace by counting braces
191+
candidate := extractBalancedJSON(text[startIdx:])
192+
if candidate != "" && json.Valid([]byte(candidate)) {
193+
return candidate
194+
}
195+
}
196+
197+
return ""
198+
}
199+
200+
// extractBalancedJSON extracts a balanced JSON object from text starting with {
201+
func extractBalancedJSON(text string) string {
202+
if len(text) == 0 || text[0] != '{' {
203+
return ""
204+
}
205+
206+
depth := 0
207+
inString := false
208+
escaped := false
209+
210+
for i, c := range text {
211+
if escaped {
212+
escaped = false
213+
continue
214+
}
215+
216+
if c == '\\' && inString {
217+
escaped = true
218+
continue
219+
}
220+
221+
if c == '"' {
222+
inString = !inString
223+
continue
224+
}
225+
226+
if inString {
227+
continue
228+
}
229+
230+
switch c {
231+
case '{':
232+
depth++
233+
case '}':
234+
depth--
235+
if depth == 0 {
236+
return text[:i+1]
237+
}
189238
}
190239
}
191240

internal/ai/conversation_manager.go

Lines changed: 52 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,9 +14,11 @@ import (
1414
)
1515

1616
const (
17-
openAiBaseURL = "https://api.openai.com/v1"
18-
openRouterBaseURL = "https://openrouter.ai/api/v1"
19-
ollamaBaseURL = "http://localhost:11434/v1"
17+
openAiBaseURL = "https://api.openai.com/v1"
18+
openRouterBaseURL = "https://openrouter.ai/api/v1"
19+
ollamaBaseURL = "http://localhost:11434/v1"
20+
maxConversations = 100 // Maximum number of conversations to keep
21+
conversationMaxAge = 24 * time.Hour // Maximum age before cleanup
2022
)
2123

2224
// ConversationManager manages ongoing AI conversations
@@ -39,6 +41,11 @@ func (cm *ConversationManager) StartConversation(provider, model, apiKey, baseUR
3941
cm.mu.Lock()
4042
defer cm.mu.Unlock()
4143

44+
// Cleanup old conversations if we have too many
45+
if len(cm.conversations) >= maxConversations {
46+
cm.cleanupLocked()
47+
}
48+
4249
conv := &AIConversation{
4350
ID: fmt.Sprintf("conv-%d", time.Now().UnixNano()),
4451
Provider: provider,
@@ -142,6 +149,48 @@ func (cm *ConversationManager) CleanupOldConversations(maxAge time.Duration) {
142149
}
143150
}
144151

152+
// cleanupLocked removes old conversations (must be called with lock held)
153+
func (cm *ConversationManager) cleanupLocked() {
154+
cutoff := time.Now().Add(-conversationMaxAge)
155+
cleaned := 0
156+
157+
// First pass: remove conversations older than maxAge
158+
for id, conv := range cm.conversations {
159+
if conv.LastActivity.Before(cutoff) {
160+
delete(cm.conversations, id)
161+
cleaned++
162+
logger.DebugfToFile("ConversationManager", "Cleaned up old conversation %s", id)
163+
}
164+
}
165+
166+
// Second pass: if still over limit, remove oldest conversations
167+
for len(cm.conversations) >= maxConversations {
168+
var oldestID string
169+
var oldestTime time.Time
170+
first := true
171+
172+
for id, conv := range cm.conversations {
173+
if first || conv.LastActivity.Before(oldestTime) {
174+
oldestID = id
175+
oldestTime = conv.LastActivity
176+
first = false
177+
}
178+
}
179+
180+
if oldestID != "" {
181+
delete(cm.conversations, oldestID)
182+
cleaned++
183+
logger.DebugfToFile("ConversationManager", "Evicted oldest conversation %s to stay under limit", oldestID)
184+
} else {
185+
break
186+
}
187+
}
188+
189+
if cleaned > 0 {
190+
logger.DebugfToFile("ConversationManager", "Cleanup complete: removed %d conversations, %d remaining", cleaned, len(cm.conversations))
191+
}
192+
}
193+
145194
// Continue continues the conversation with user input (or empty string for continuation)
146195
func (conv *AIConversation) Continue(ctx context.Context, userInput string) (*AIResult, *InteractionRequest, error) {
147196
conv.LastActivity = time.Now()

internal/ai/search_index_manager.go

Lines changed: 31 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -21,12 +21,14 @@ type FuzzyMatch struct {
2121
// SearchIndexManager manages the search index for fuzzy matching
2222
// This is separate from schema cache to optimize performance
2323
type SearchIndexManager struct {
24-
cache *db.SchemaCache
25-
tableIndex map[string]*TableSearchEntry // keyspace.table -> search entry
26-
lastIndexBuild time.Time
27-
indexTTL time.Duration
28-
mu sync.RWMutex
29-
buildInProgress bool
24+
cache *db.SchemaCache
25+
tableIndex map[string]*TableSearchEntry // keyspace.table -> search entry
26+
lastIndexBuild time.Time
27+
lastAccessUpdate time.Time // Last time we updated access timestamps (for debouncing)
28+
indexTTL time.Duration
29+
mu sync.RWMutex
30+
buildInProgress bool
31+
pendingAccessBatch []FuzzyMatch // Batched access updates
3032
}
3133

3234
// TableSearchEntry contains search metadata for a table
@@ -132,14 +134,13 @@ func (sim *SearchIndexManager) FindTables(query string, limit int) []FuzzyMatch
132134
logger.DebugfToFile("SearchIndexManager", "Failed to build index: %v", err)
133135
}
134136

135-
sim.mu.RLock()
136-
defer sim.mu.RUnlock()
137-
138137
queryLower := strings.ToLower(query)
139138
queryTokens := tokenizeTableName(query)
140139

141140
var matches []FuzzyMatch
142141

142+
// Hold read lock only during the search
143+
sim.mu.RLock()
143144
for _, entry := range sim.tableIndex {
144145
score := calculateFuzzyScore(queryLower, queryTokens, entry)
145146

@@ -152,6 +153,7 @@ func (sim *SearchIndexManager) FindTables(query string, limit int) []FuzzyMatch
152153
})
153154
}
154155
}
156+
sim.mu.RUnlock()
155157

156158
// Sort by score (highest first)
157159
sortFuzzyMatches(matches)
@@ -161,24 +163,39 @@ func (sim *SearchIndexManager) FindTables(query string, limit int) []FuzzyMatch
161163
matches = matches[:limit]
162164
}
163165

164-
// Update last accessed time for matched entries
165-
go sim.updateLastAccessed(matches)
166+
// Debounce last accessed time updates (only update every 30 seconds)
167+
sim.maybeUpdateLastAccessed(matches)
166168

167169
return matches
168170
}
169171

170-
// updateLastAccessed updates the last accessed time for matched entries
171-
func (sim *SearchIndexManager) updateLastAccessed(matches []FuzzyMatch) {
172+
// accessUpdateDebounce is the minimum interval between access timestamp updates
173+
const accessUpdateDebounce = 30 * time.Second
174+
175+
// maybeUpdateLastAccessed updates access timestamps with debouncing
176+
// to avoid spawning goroutines for every FindTables call
177+
func (sim *SearchIndexManager) maybeUpdateLastAccessed(matches []FuzzyMatch) {
172178
sim.mu.Lock()
173179
defer sim.mu.Unlock()
174180

181+
// Batch the matches
182+
sim.pendingAccessBatch = append(sim.pendingAccessBatch, matches...)
183+
184+
// Check if we should flush the batch
185+
if time.Since(sim.lastAccessUpdate) < accessUpdateDebounce {
186+
return
187+
}
188+
189+
// Flush the batch
175190
now := time.Now()
176-
for _, match := range matches {
191+
for _, match := range sim.pendingAccessBatch {
177192
key := fmt.Sprintf("%s.%s", match.Keyspace, match.Table)
178193
if entry, ok := sim.tableIndex[key]; ok {
179194
entry.LastAccessed = now
180195
}
181196
}
197+
sim.pendingAccessBatch = nil
198+
sim.lastAccessUpdate = now
182199
}
183200

184201
// calculateFuzzyScore calculates the fuzzy match score

internal/batch/executor.go

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,8 @@ func NewExecutor(options *Options, writer io.Writer) (*Executor, error) {
134134
// Create session manager for tracking keyspace changes
135135
sessionMgr := session.NewManager(cfg)
136136
if cfg.Keyspace != "" {
137-
sessionMgr.SetKeyspace(cfg.Keyspace)
137+
// SetKeyspace validates the keyspace name, but config values should already be valid
138+
_ = sessionMgr.SetKeyspace(cfg.Keyspace)
138139
}
139140

140141
// Initialize router with session manager
@@ -221,9 +222,9 @@ func (e *Executor) Execute(cql string) error {
221222
keyspaceName := strings.TrimPrefix(v, "Now using keyspace ")
222223
keyspaceName = strings.TrimSpace(keyspaceName)
223224

224-
// Update the session manager
225+
// Update the session manager (keyspace already validated by Cassandra)
225226
if e.sessionManager != nil {
226-
e.sessionManager.SetKeyspace(keyspaceName)
227+
_ = e.sessionManager.SetKeyspace(keyspaceName)
227228
}
228229

229230
// Update the database session's keyspace

0 commit comments

Comments
 (0)