2 Commits

Author SHA1 Message Date
Taylor Eernisse
435a208c93 perf: eliminate unnecessary clones and pre-allocate collections
Three micro-optimizations with zero behavioral change:

1. timeline_collect.rs: Reorder format!() before enum construction so
   the owned String moves into the variant directly, eliminating
   .clone() on state, label, and milestone strings in StateChanged,
   LabelAdded/Removed, and MilestoneSet/Removed event paths.

2. pipeline.rs: Use Arc<str> for doc_hash shared across a document's
   chunks instead of cloning the full String per chunk. Also remove
   redundant embed_buf.reserve() since extend_from_slice already
   handles growth and the buffer is reused across iterations.

3. rrf.rs: Pre-allocate HashMap with combined vector+fts result count
   via with_capacity() to avoid rehashing during RRF score accumulation.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-08 08:08:14 -05:00
Taylor Eernisse
cc11d3e5a0 fix: peer review — 5 correctness bugs across who, db, lock, embedding, main
Comprehensive peer code review identified and fixed the following:

1. who.rs: @-prefixed path routing used `target` (with @) instead of
   `clean` (stripped) when checking for '/' and passing to Expert mode,
   causing `lore who @src/auth/` to silently return zero results because
   the SQL LIKE matched against `@src/auth/%` which never exists.

2. db.rs: After ROLLBACK TO savepoint on migration failure, the savepoint
   was never RELEASEd, leaving it active on the connection. Fixed in both
   run_migrations() and run_migrations_from_dir().

3. lock.rs: Multiple acquire() calls (e.g. re-acquiring a stale lock)
   replaced the heartbeat_handle without stopping the old thread, causing
   two concurrent heartbeat writers competing on the same lock row. Now
   signals the old thread to stop and joins it before spawning a new one.

4. chunk_ids.rs: encode_rowid() had no guard for chunk_index >= 1000
   (CHUNK_ROWID_MULTIPLIER), which would cause rowid collisions between
   adjacent documents. Added range assertion [0, 1000).

5. main.rs: Fallback JSON error formatting in handle_auth_test
   interpolated LoreError Display output without escaping quotes or
   backslashes, potentially producing malformed JSON for robot-mode
   consumers. Now escapes both characters before interpolation.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-08 08:07:59 -05:00
8 changed files with 46 additions and 35 deletions

View File

@@ -54,9 +54,9 @@ fn resolve_mode<'a>(args: &'a WhoArgs) -> Result<WhoMode<'a>> {
// Disambiguation: if target contains '/', it's a file path. // Disambiguation: if target contains '/', it's a file path.
// GitLab usernames never contain '/'. // GitLab usernames never contain '/'.
// Root files (no '/') require --path. // Root files (no '/') require --path.
if target.contains('/') { if clean.contains('/') {
return Ok(WhoMode::Expert { return Ok(WhoMode::Expert {
path: normalize_repo_path(target), path: normalize_repo_path(clean),
}); });
} }
return Ok(WhoMode::Workload { username: clean }); return Ok(WhoMode::Workload { username: clean });

View File

@@ -140,6 +140,7 @@ pub fn run_migrations(conn: &Connection) -> Result<()> {
} }
Err(e) => { Err(e) => {
let _ = conn.execute_batch(&format!("ROLLBACK TO {}", savepoint_name)); let _ = conn.execute_batch(&format!("ROLLBACK TO {}", savepoint_name));
let _ = conn.execute_batch(&format!("RELEASE {}", savepoint_name));
return Err(LoreError::MigrationFailed { return Err(LoreError::MigrationFailed {
version, version,
message: e.to_string(), message: e.to_string(),
@@ -216,6 +217,7 @@ pub fn run_migrations_from_dir(conn: &Connection, migrations_dir: &Path) -> Resu
} }
Err(e) => { Err(e) => {
let _ = conn.execute_batch(&format!("ROLLBACK TO {}", savepoint_name)); let _ = conn.execute_batch(&format!("ROLLBACK TO {}", savepoint_name));
let _ = conn.execute_batch(&format!("RELEASE {}", savepoint_name));
return Err(LoreError::MigrationFailed { return Err(LoreError::MigrationFailed {
version, version,
message: e.to_string(), message: e.to_string(),

View File

@@ -135,6 +135,13 @@ impl AppLock {
} }
fn start_heartbeat(&mut self) { fn start_heartbeat(&mut self) {
// Stop any existing heartbeat thread before starting a new one
if let Some(handle) = self.heartbeat_handle.take() {
self.released.store(true, Ordering::SeqCst);
let _ = handle.join();
self.released.store(false, Ordering::SeqCst);
}
let name = self.name.clone(); let name = self.name.clone();
let owner = self.owner.clone(); let owner = self.owner.clone();
let interval = Duration::from_millis(self.heartbeat_interval_ms); let interval = Duration::from_millis(self.heartbeat_interval_ms);

View File

@@ -146,16 +146,15 @@ fn collect_state_events(
continue; continue;
} }
let summary = format!("State changed to {state}");
events.push(TimelineEvent { events.push(TimelineEvent {
timestamp: created_at, timestamp: created_at,
entity_type: entity.entity_type.clone(), entity_type: entity.entity_type.clone(),
entity_id: entity.entity_id, entity_id: entity.entity_id,
entity_iid: entity.entity_iid, entity_iid: entity.entity_iid,
project_path: entity.project_path.clone(), project_path: entity.project_path.clone(),
event_type: TimelineEventType::StateChanged { event_type: TimelineEventType::StateChanged { state },
state: state.clone(), summary,
},
summary: format!("State changed to {state}"),
actor, actor,
url: None, url: None,
is_seed, is_seed,
@@ -195,18 +194,14 @@ fn collect_label_events(
let label = label_name.unwrap_or_else(|| "[deleted label]".to_owned()); let label = label_name.unwrap_or_else(|| "[deleted label]".to_owned());
let (event_type, summary) = match action.as_str() { let (event_type, summary) = match action.as_str() {
"add" => ( "add" => {
TimelineEventType::LabelAdded { let summary = format!("Label added: {label}");
label: label.clone(), (TimelineEventType::LabelAdded { label }, summary)
}, }
format!("Label added: {label}"), "remove" => {
), let summary = format!("Label removed: {label}");
"remove" => ( (TimelineEventType::LabelRemoved { label }, summary)
TimelineEventType::LabelRemoved { }
label: label.clone(),
},
format!("Label removed: {label}"),
),
_ => continue, _ => continue,
}; };
@@ -257,18 +252,14 @@ fn collect_milestone_events(
let milestone = milestone_title.unwrap_or_else(|| "[deleted milestone]".to_owned()); let milestone = milestone_title.unwrap_or_else(|| "[deleted milestone]".to_owned());
let (event_type, summary) = match action.as_str() { let (event_type, summary) = match action.as_str() {
"add" => ( "add" => {
TimelineEventType::MilestoneSet { let summary = format!("Milestone set: {milestone}");
milestone: milestone.clone(), (TimelineEventType::MilestoneSet { milestone }, summary)
}, }
format!("Milestone set: {milestone}"), "remove" => {
), let summary = format!("Milestone removed: {milestone}");
"remove" => ( (TimelineEventType::MilestoneRemoved { milestone }, summary)
TimelineEventType::MilestoneRemoved { }
milestone: milestone.clone(),
},
format!("Milestone removed: {milestone}"),
),
_ => continue, _ => continue,
}; };

View File

@@ -1,6 +1,10 @@
pub const CHUNK_ROWID_MULTIPLIER: i64 = 1000; pub const CHUNK_ROWID_MULTIPLIER: i64 = 1000;
pub fn encode_rowid(document_id: i64, chunk_index: i64) -> i64 { pub fn encode_rowid(document_id: i64, chunk_index: i64) -> i64 {
assert!(
(0..CHUNK_ROWID_MULTIPLIER).contains(&chunk_index),
"chunk_index {chunk_index} out of range [0, {CHUNK_ROWID_MULTIPLIER})"
);
document_id document_id
.checked_mul(CHUNK_ROWID_MULTIPLIER) .checked_mul(CHUNK_ROWID_MULTIPLIER)
.and_then(|v| v.checked_add(chunk_index)) .and_then(|v| v.checked_add(chunk_index))

View File

@@ -1,4 +1,5 @@
use std::collections::{HashMap, HashSet}; use std::collections::{HashMap, HashSet};
use std::sync::Arc;
use futures::future::join_all; use futures::future::join_all;
use rusqlite::Connection; use rusqlite::Connection;
@@ -28,7 +29,7 @@ struct ChunkWork {
doc_id: i64, doc_id: i64,
chunk_index: usize, chunk_index: usize,
total_chunks: usize, total_chunks: usize,
doc_hash: String, doc_hash: Arc<str>,
chunk_hash: String, chunk_hash: String,
text: String, text: String,
} }
@@ -212,12 +213,13 @@ async fn embed_page(
chunks_needed.insert(doc.document_id, total_chunks); chunks_needed.insert(doc.document_id, total_chunks);
let doc_hash: Arc<str> = Arc::from(doc.content_hash.as_str());
for (chunk_index, text) in chunks { for (chunk_index, text) in chunks {
all_chunks.push(ChunkWork { all_chunks.push(ChunkWork {
doc_id: doc.document_id, doc_id: doc.document_id,
chunk_index, chunk_index,
total_chunks, total_chunks,
doc_hash: doc.content_hash.clone(), doc_hash: Arc::clone(&doc_hash),
chunk_hash: sha256_hash(&text), chunk_hash: sha256_hash(&text),
text, text,
}); });
@@ -501,7 +503,6 @@ fn store_embedding(
let rowid = encode_rowid(doc_id, chunk_index as i64); let rowid = encode_rowid(doc_id, chunk_index as i64);
embed_buf.clear(); embed_buf.clear();
embed_buf.reserve(embedding.len() * 4);
for f in embedding { for f in embedding {
embed_buf.extend_from_slice(&f.to_le_bytes()); embed_buf.extend_from_slice(&f.to_le_bytes());
} }

View File

@@ -1217,7 +1217,12 @@ async fn handle_auth_test(
eprintln!( eprintln!(
"{}", "{}",
serde_json::to_string(&output).unwrap_or_else(|_| { serde_json::to_string(&output).unwrap_or_else(|_| {
format!(r#"{{"error":{{"code":"{}","message":"{}"}}}}"#, e.code(), e) let msg = e.to_string().replace('\\', "\\\\").replace('"', "\\\"");
format!(
r#"{{"error":{{"code":"{}","message":"{}"}}}}"#,
e.code(),
msg
)
}) })
); );
} else { } else {

View File

@@ -15,7 +15,8 @@ pub fn rank_rrf(vector_results: &[(i64, f64)], fts_results: &[(i64, f64)]) -> Ve
return Vec::new(); return Vec::new();
} }
let mut scores: HashMap<i64, (f64, Option<usize>, Option<usize>)> = HashMap::new(); let mut scores: HashMap<i64, (f64, Option<usize>, Option<usize>)> =
HashMap::with_capacity(vector_results.len() + fts_results.len());
for (i, &(doc_id, _)) in vector_results.iter().enumerate() { for (i, &(doc_id, _)) in vector_results.iter().enumerate() {
let rank = i + 1; let rank = i + 1;