Skip to content

Commit 19f7456

Browse files
Revert "feat: Add conditional compilation for Knowledge command using… (#2520)
* Revert "feat: Add conditional compilation for Knowledge command using feature flag (#2513)" This reverts commit 0e034b4. * feat: enhance semantic search client with model validation - Add model_validator.rs for input validation - Update async_implementation.rs and hosted_model_client.rs with improvements - Add new dependency in Cargo.toml - Export model validator in lib.rs as message and push to the remote reenables_knowledge_tool * Update semantic search client implementation --------- Co-authored-by: Kenneth S. <[email protected]>
1 parent f27a0bb commit 19f7456

File tree

12 files changed

+188
-105
lines changed

12 files changed

+188
-105
lines changed

Cargo.lock

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

crates/chat-cli/Cargo.toml

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ workspace = true
1414
[features]
1515
default = []
1616
wayland = ["arboard/wayland-data-control"]
17-
knowledge = []
1817

1918
[[bin]]
2019
name = "test_mcp_server"

crates/chat-cli/src/cli/chat/cli/knowledge.rs

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,6 @@ use crate::cli::chat::{
1919
ChatSession,
2020
ChatState,
2121
};
22-
#[cfg(feature = "knowledge")]
2322
use crate::database::settings::Setting;
2423
use crate::os::Os;
2524
use crate::util::knowledge_store::KnowledgeStore;
@@ -70,19 +69,10 @@ impl KnowledgeSubcommand {
7069
}
7170

7271
fn is_feature_enabled(os: &Os) -> bool {
73-
// Feature is only available when compiled with the knowledge feature flag
74-
#[cfg(feature = "knowledge")]
75-
{
76-
os.database
77-
.settings
78-
.get_bool(Setting::EnabledKnowledge)
79-
.unwrap_or(false)
80-
}
81-
#[cfg(not(feature = "knowledge"))]
82-
{
83-
let _ = os; // Suppress unused variable warning
84-
false
85-
}
72+
os.database
73+
.settings
74+
.get_bool(Setting::EnabledKnowledge)
75+
.unwrap_or(false)
8676
}
8777

8878
fn write_feature_disabled_message(session: &mut ChatSession) -> Result<(), std::io::Error> {

crates/chat-cli/src/cli/chat/cli/mod.rs

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@ pub mod compact;
33
pub mod context;
44
pub mod editor;
55
pub mod hooks;
6-
#[cfg(feature = "knowledge")]
76
pub mod knowledge;
87
pub mod mcp;
98
pub mod model;
@@ -20,7 +19,6 @@ use compact::CompactArgs;
2019
use context::ContextSubcommand;
2120
use editor::EditorArgs;
2221
use hooks::HooksArgs;
23-
#[cfg(feature = "knowledge")]
2422
use knowledge::KnowledgeSubcommand;
2523
use mcp::McpArgs;
2624
use model::ModelArgs;
@@ -58,9 +56,9 @@ pub enum SlashCommand {
5856
/// Manage context files for the chat session
5957
#[command(subcommand)]
6058
Context(ContextSubcommand),
61-
/// (Beta) Manage knowledge base for persistent context storage
62-
#[cfg(feature = "knowledge")]
63-
#[command(subcommand)]
59+
/// (Beta) Manage knowledge base for persistent context storage. Requires "q settings
60+
/// chat.enableKnowledge true"
61+
#[command(subcommand, hide = true)]
6462
Knowledge(KnowledgeSubcommand),
6563
/// Open $EDITOR (defaults to vi) to compose a prompt
6664
#[command(name = "editor")]
@@ -119,7 +117,6 @@ impl SlashCommand {
119117
})
120118
},
121119
Self::Context(args) => args.execute(os, session).await,
122-
#[cfg(feature = "knowledge")]
123120
Self::Knowledge(subcommand) => subcommand.execute(os, session).await,
124121
Self::PromptEditor(args) => args.execute(session).await,
125122
Self::Compact(args) => args.execute(os, session).await,
@@ -159,7 +156,6 @@ impl SlashCommand {
159156
Self::Agent(_) => "agent",
160157
Self::Profile => "profile",
161158
Self::Context(_) => "context",
162-
#[cfg(feature = "knowledge")]
163159
Self::Knowledge(_) => "knowledge",
164160
Self::PromptEditor(_) => "editor",
165161
Self::Compact(_) => "compact",
@@ -182,7 +178,6 @@ impl SlashCommand {
182178
match self {
183179
SlashCommand::Agent(sub) => Some(sub.name()),
184180
SlashCommand::Context(sub) => Some(sub.name()),
185-
#[cfg(feature = "knowledge")]
186181
SlashCommand::Knowledge(sub) => Some(sub.name()),
187182
SlashCommand::Tools(arg) => arg.subcommand_name(),
188183
SlashCommand::Prompts(arg) => arg.subcommand_name(),

crates/chat-cli/src/cli/chat/tools/knowledge.rs

Lines changed: 4 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,6 @@ use crate::cli::agent::{
1717
Agent,
1818
PermissionEvalResult,
1919
};
20-
#[cfg(feature = "knowledge")]
2120
use crate::database::settings::Setting;
2221
use crate::os::Os;
2322
use crate::util::knowledge_store::KnowledgeStore;
@@ -88,19 +87,10 @@ pub struct KnowledgeCancel {
8887
impl Knowledge {
8988
/// Checks if the knowledge feature is enabled in settings
9089
pub fn is_enabled(os: &Os) -> bool {
91-
// Feature is only available when compiled with the knowledge feature flag
92-
#[cfg(feature = "knowledge")]
93-
{
94-
os.database
95-
.settings
96-
.get_bool(Setting::EnabledKnowledge)
97-
.unwrap_or(false)
98-
}
99-
#[cfg(not(feature = "knowledge"))]
100-
{
101-
let _ = os; // Suppress unused variable warning
102-
false
103-
}
90+
os.database
91+
.settings
92+
.get_bool(Setting::EnabledKnowledge)
93+
.unwrap_or(false)
10494
}
10595

10696
pub async fn validate(&mut self, os: &Os) -> Result<()> {

crates/chat-cli/src/util/knowledge_store.rs

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,6 @@ impl KnowledgeStore {
159159
}
160160

161161
/// Clear all contexts immediately (synchronous operation)
162-
#[allow(dead_code)]
163162
pub async fn clear_immediate(&mut self) -> Result<String, String> {
164163
match self.client.clear_all_immediate().await {
165164
Ok(count) => Ok(format!("✅ Successfully cleared {} knowledge base entries", count)),

crates/semantic-search-client/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -36,6 +36,7 @@ anyhow = "1.0"
3636
reqwest = { workspace = true }
3737
zip = { version = "4.3.0", default-features = false, features = ["deflate", "time"] }
3838
tokio-stream = "0.1.17"
39+
sha2 = "0.10.9"
3940

4041
# Candle dependencies - not used on Linux ARM
4142
[target.'cfg(not(all(target_os = "linux", target_arch = "aarch64")))'.dependencies]

crates/semantic-search-client/src/client/async_implementation.rs

Lines changed: 16 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -108,10 +108,7 @@ impl AsyncSemanticSearchClient {
108108
use crate::embedding::ModelType;
109109

110110
let model_config = ModelType::default().get_config();
111-
let (model_path, tokenizer_path) = model_config.get_local_paths();
112-
if model_path.exists() && tokenizer_path.exists() {
113-
return Ok(());
114-
}
111+
let (model_path, _tokenizer_path) = model_config.get_local_paths();
115112

116113
// Create model directory if it doesn't exist
117114
if let Some(parent) = model_path.parent() {
@@ -120,31 +117,25 @@ impl AsyncSemanticSearchClient {
120117
.map_err(SemanticSearchError::IoError)?;
121118
}
122119

123-
// Check if files already exist
124-
let model_exists = tokio::fs::try_exists(&model_path).await.unwrap_or(false);
125-
let tokenizer_exists = tokio::fs::try_exists(&tokenizer_path).await.unwrap_or(false);
126-
127-
if !model_exists || !tokenizer_exists {
128-
debug!("Downloading model files for {}...", model_config.name);
120+
debug!("Reviewing model files for {}...", model_config.name);
129121

130-
// Get the target directory (parent of model_path, which should be the model directory)
131-
let target_dir = model_path
132-
.parent()
133-
.ok_or_else(|| SemanticSearchError::EmbeddingError("Invalid model path".to_string()))?;
122+
// Get the target directory (parent of model_path, which should be the model directory)
123+
let target_dir = model_path
124+
.parent()
125+
.ok_or_else(|| SemanticSearchError::EmbeddingError("Invalid model path".to_string()))?;
134126

135-
// Get the hosted models base URL from config
136-
let semantic_config = crate::config::get_config();
137-
let base_url = &semantic_config.hosted_models_base_url;
127+
// Get the hosted models base URL from config
128+
let semantic_config = crate::config::get_config();
129+
let base_url = &semantic_config.hosted_models_base_url;
138130

139-
// Create hosted model client and download with progress bar
140-
let client = HostedModelClient::new(base_url.clone());
141-
client
142-
.ensure_model(&model_config.name, target_dir)
143-
.await
144-
.map_err(|e| SemanticSearchError::EmbeddingError(format!("Failed to download model: {}", e)))?;
131+
// Create hosted model client and download with progress bar
132+
let client = HostedModelClient::new(base_url.clone());
133+
client
134+
.ensure_model(&model_config, target_dir)
135+
.await
136+
.map_err(|e| SemanticSearchError::EmbeddingError(format!("Failed to download model: {}", e)))?;
145137

146-
debug!("Model download completed for {}", model_config.name);
147-
}
138+
debug!("Model download completed for {}", model_config.name);
148139
},
149140
EmbeddingType::BM25 => {
150141
// BM25 doesn't require model downloads

0 commit comments

Comments
 (0)