From 5b20c9703b8c7ee43fd666f3cfa3129fb026fc22 Mon Sep 17 00:00:00 2001 From: krishpinto <161019503+krishpinto@users.noreply.github.com> Date: Tue, 30 Jun 2026 13:07:19 +0530 Subject: [PATCH 1/2] fix(core): correct project-scoped forget, dim-safe cosine, precise question detection MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three correctness fixes found auditing the engine: - forget(): the processed_files cleanup used `LIKE '%slug%'`, an unanchored substring match. It also cleared sibling projects (forgetting `…-grasp` matched `…-grasp-core`) and treated `_`/`%` in a slug as wildcards, forcing needless re-imports. Now matches the project's transcript directory as an escaped, anchored prefix (`escape_like` handles Windows backslashes). - cosine(): `zip` silently computed a truncated dot product when vector lengths differed. The `embeddings` table stores a per-row `dim`, so a future model/DIM change can leave stale vectors of another length — those must not corrupt ranking or semantic edges. Now returns 0.0 on a dimension mismatch. - is_question(): a bare `contains("what ")` flagged "somewhat" (and similar) as questions. Now matches interrogatives only at a word boundary. Each fix has unit-test coverage; full grasp-core suite green. Co-Authored-By: Claude Opus 4.8 --- crates/grasp-core/src/embed.rs | 22 +++++++++++++++ crates/grasp-core/src/extractor.rs | 36 +++++++++++++++++++++++- crates/grasp-core/src/lib.rs | 45 ++++++++++++++++++++++++++++-- 3 files changed, 100 insertions(+), 3 deletions(-) diff --git a/crates/grasp-core/src/embed.rs b/crates/grasp-core/src/embed.rs index 3585287..2747a13 100644 --- a/crates/grasp-core/src/embed.rs +++ b/crates/grasp-core/src/embed.rs @@ -146,7 +146,16 @@ impl Embedder { } /// Cosine similarity of two equal-length, L2-normalized vectors (== dot product). +/// +/// Returns 0.0 when the dimensions differ rather than silently computing a +/// truncated dot product over the shorter length — which `zip` would otherwise +/// do. Mismatched dimensions are reachable: the `embeddings` table stores a +/// per-row `dim`, so a future model/`DIM` change can leave stale vectors of a +/// different length that must not corrupt ranking or semantic edges. pub fn cosine(a: &[f32], b: &[f32]) -> f32 { + if a.len() != b.len() { + return 0.0; + } a.iter().zip(b).map(|(x, y)| x * y).sum() } @@ -154,6 +163,19 @@ pub fn cosine(a: &[f32], b: &[f32]) -> f32 { mod tests { use super::*; + #[test] + fn cosine_of_mismatched_dimensions_is_zero() { + // A stale embedding of a different length must not yield a bogus + // (truncated) similarity that could outrank correctly-sized vectors. + assert_eq!(cosine(&[1.0, 0.0, 0.0], &[1.0, 0.0]), 0.0); + } + + #[test] + fn cosine_of_identical_unit_vectors_is_one() { + let v = [0.6_f32, 0.8]; + assert!((cosine(&v, &v) - 1.0).abs() < 1e-6); + } + /// Sanity check that the candle MiniLM port produces *meaningful* vectors: /// a query must be closer to a related sentence than an unrelated one, and /// vectors must be the right shape and L2-normalized. diff --git a/crates/grasp-core/src/extractor.rs b/crates/grasp-core/src/extractor.rs index d43c26d..d994377 100644 --- a/crates/grasp-core/src/extractor.rs +++ b/crates/grasp-core/src/extractor.rs @@ -309,9 +309,31 @@ fn is_question(text: &str) -> bool { return true; } let lower = t.to_lowercase(); + // Match the interrogative only at a word boundary, so substrings like + // "somewhat" (contains "what ") or "showhow" don't masquerade as questions. ["why ", "how ", "should ", "what ", "when should"] .iter() - .any(|k| lower.contains(k)) + .any(|k| contains_at_word_boundary(&lower, k)) +} + +/// Does `needle` occur in `haystack` at a word boundary — i.e. at the start of +/// the string or immediately after a non-alphanumeric character? Avoids the +/// false positives a bare `contains` produces on embedded substrings. +fn contains_at_word_boundary(haystack: &str, needle: &str) -> bool { + let mut from = 0; + while let Some(rel) = haystack[from..].find(needle) { + let abs = from + rel; + let boundary = abs == 0 + || !haystack[..abs] + .chars() + .next_back() + .is_some_and(|c| c.is_alphanumeric()); + if boundary { + return true; + } + from = abs + needle.len(); + } + false } /// Leading sentences of a block of text, up to `max_chars` — a compact summary @@ -436,6 +458,18 @@ mod tests { assert!(chunks.iter().any(|c| c.chunk_type == ChunkType::ErrorResolution)); } + #[test] + fn question_detection_respects_word_boundaries() { + // Real interrogatives at a word boundary are questions. + assert!(is_question("how do we cache the tokens")); + assert!(is_question("what database are we using")); + assert!(is_question("when should we embed the chunks")); + // Embedded substrings must NOT count (this was a false positive: the + // word "somewhat" contains "what "). + assert!(!is_question("this loop is somewhat slow today")); + assert!(!is_question("the showhow demo build finished cleanly")); + } + #[test] fn extracts_user_question_as_context() { let entries = vec![Entry::User { diff --git a/crates/grasp-core/src/lib.rs b/crates/grasp-core/src/lib.rs index 1ff0a29..5ef3d6a 100644 --- a/crates/grasp-core/src/lib.rs +++ b/crates/grasp-core/src/lib.rs @@ -22,6 +22,20 @@ pub use model::{Chunk, ChunkType, Entry, SearchHit}; use anyhow::Result; use rusqlite::Connection; +/// Escape SQL `LIKE` wildcards (`%`, `_`) and the escape character (`\`) so a +/// string can be used as a literal prefix in a `LIKE ? ESCAPE '\'` clause — +/// notably so backslashes in Windows transcript paths aren't misread. +fn escape_like(s: &str) -> String { + let mut out = String::with_capacity(s.len()); + for c in s.chars() { + if matches!(c, '\\' | '%' | '_') { + out.push('\\'); + } + out.push(c); + } + out +} + /// Convenience handle bundling the resolved config with an open DB connection. pub struct Grasp { pub config: Config, @@ -163,9 +177,22 @@ impl Grasp { .execute("DELETE FROM chunks WHERE project = ?1", [project])?; self.conn .execute("DELETE FROM projects WHERE slug = ?1", [project])?; + // Remove processed-file records for *this* project only. Match the + // project's transcript directory as an anchored prefix. A bare + // `%slug%` substring also matched sibling projects (e.g. forgetting + // `…-grasp` cleared `…-grasp-core`) and treated `_`/`%` in a slug as + // wildcards — both forced needless re-imports of other projects. + let mut prefix = self + .config + .claude_projects_dir + .join(project) + .to_string_lossy() + .into_owned(); + prefix.push(std::path::MAIN_SEPARATOR); + let pattern = format!("{}%", escape_like(&prefix)); self.conn.execute( - "DELETE FROM processed_files WHERE file_path LIKE ?1", - [format!("%{project}%")], + "DELETE FROM processed_files WHERE file_path LIKE ?1 ESCAPE '\\'", + [pattern], )?; let dir = self.config.memory_project_dir(project); if dir.exists() { @@ -286,3 +313,17 @@ impl Grasp { Ok(added.unwrap_or(0)) } } + +#[cfg(test)] +mod tests { + use super::escape_like; + + #[test] + fn escape_like_escapes_wildcards_and_backslashes() { + assert_eq!(escape_like("a_b%c"), r"a\_b\%c"); + // Windows path separators must be escaped so they stay literal. + assert_eq!(escape_like(r"C:\projects\grasp"), r"C:\\projects\\grasp"); + // Ordinary text is untouched. + assert_eq!(escape_like("c--projects-grasp"), "c--projects-grasp"); + } +} From 646358ba10473b94ecae6b5bd3b00de9a8edc596 Mon Sep 17 00:00:00 2001 From: krishpinto <161019503+krishpinto@users.noreply.github.com> Date: Tue, 30 Jun 2026 13:16:28 +0530 Subject: [PATCH 2/2] fix(core): base forget() cleanup on the stored import path (closes #26) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses CodeRabbit's Major finding on PR #25. The processed_files cleanup rebuilt its prefix from claude_projects_dir, which misses transcripts imported from a custom path (`grasp import --path …`) — their processed_files rows live outside claude_projects_dir, so forgetting such a project left stale rows and re-imports were wrongly skipped. Now read projects.path (the real import directory recorded at import time) before deleting the project row, and use it as the LIKE prefix base, falling back to claude_projects_dir.join(slug) when no row exists. Co-Authored-By: Claude Opus 4.8 --- crates/grasp-core/src/lib.rs | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/crates/grasp-core/src/lib.rs b/crates/grasp-core/src/lib.rs index 5ef3d6a..a620cd2 100644 --- a/crates/grasp-core/src/lib.rs +++ b/crates/grasp-core/src/lib.rs @@ -20,7 +20,7 @@ pub use import::{import_all, import_file, ImportReport}; pub use model::{Chunk, ChunkType, Entry, SearchHit}; use anyhow::Result; -use rusqlite::Connection; +use rusqlite::{Connection, OptionalExtension}; /// Escape SQL `LIKE` wildcards (`%`, `_`) and the escape character (`\`) so a /// string can be used as a literal prefix in a `LIKE ? ESCAPE '\'` clause — @@ -175,6 +175,18 @@ impl Grasp { let removed = self .conn .execute("DELETE FROM chunks WHERE project = ?1", [project])?; + // The real import directory, if recorded — transcripts can be imported + // from a custom path (`grasp import --path …`), so `processed_files` + // rows may live outside `claude_projects_dir` (issue #26). Read it + // *before* deleting the project row. + let stored_path: Option = self + .conn + .query_row( + "SELECT path FROM projects WHERE slug = ?1", + [project], + |row| row.get(0), + ) + .optional()?; self.conn .execute("DELETE FROM projects WHERE slug = ?1", [project])?; // Remove processed-file records for *this* project only. Match the @@ -182,10 +194,9 @@ impl Grasp { // `%slug%` substring also matched sibling projects (e.g. forgetting // `…-grasp` cleared `…-grasp-core`) and treated `_`/`%` in a slug as // wildcards — both forced needless re-imports of other projects. - let mut prefix = self - .config - .claude_projects_dir - .join(project) + let mut prefix = stored_path + .map(std::path::PathBuf::from) + .unwrap_or_else(|| self.config.claude_projects_dir.join(project)) .to_string_lossy() .into_owned(); prefix.push(std::path::MAIN_SEPARATOR);