feat(rag): add driver/attached fields, validation floors and force-reingest
Phase 1 of the RAG driver abstraction (design doc sections 5.1-5.5a). Data model: - Add `driver: String` (serde default "yaml" via RagData::default_driver) and `attached: bool` as the first two fields of RagData, so driver metadata sits at the top of each RAG YAML. Old files without them load unchanged. - Add `#[serde(default)]` to the non-Option fields so a minimal attached-RAG YAML deserializes, and add `skip_serializing_if` to `vectors` so an empty map renders no `vectors:` key. - Add a hand-written `impl Default for RagData` delegating to `RagData::new()`. It is deliberately not derived: a derived impl yields `driver: ""`, which is not a valid driver string. Validation (the price of the new serde defaults): - Add `RagData::validate()`, called from `Rag::load()` after deserialization. It enforces the (driver, attached) matrix and, critically, numeric floors that the new defaults would otherwise mask: `top_k >= 1` unconditionally (a 0 makes every query return nothing, silently), and `chunk_size >= 1` plus `chunk_overlap < chunk_size` when not attached (a 0 chunk_size is a real divide-by-zero panic while sizing embedding batches). - Reject `.set rag_top_k 0` at the setter, before the set/update fork. Without this, the new load-time floor turns one keystroke into an unloadable RAG: the setter saves immediately and no dot-command can reach the file again. Rebuild actually re-embeds now: - `.rebuild rag` and `--rebuild-rag` previously re-scanned paths and re-embedded nothing, because the content-hash skip fired regardless of the refresh flag. Extract that decision into a module-level `find_hash_skip()` free function and thread a `force_reingest` flag through `sync_documents()` and `refresh_document_paths()`, set true only from `rebuild_rag()`. `.edit rag-docs` stays incremental. Re-embedding costs time and API spend, so `rebuild_rag()` now prints a one-line file-count warning first (no prompt: the path is reachable from a non-interactive CLI flag). Attached-RAG guards: - Block `.rebuild rag` / `--rebuild-rag` and `.edit rag-docs` on attached RAGs, which Coyote did not index and whose source documents it does not own. - Add `Rag::driver()`, `Rag::is_attached()` and `Rag::file_count()`, and surface driver/attached through `Rag::export()` so `.info rag` shows them. Adds 12 unit tests (1299 -> 1311), including the two gate tests pinning that a forced re-ingest does not hash-skip while an ordinary refresh still does.
This commit is contained in:
@@ -2773,7 +2773,12 @@ impl RequestContext {
|
||||
}
|
||||
}
|
||||
"rag_top_k" => {
|
||||
let value = value.parse().with_context(|| "Invalid value")?;
|
||||
let value: usize = value.parse().with_context(|| "Invalid value")?;
|
||||
if value == 0 {
|
||||
bail!(
|
||||
"rag_top_k must be >= 1; a top_k of 0 makes every query return no results."
|
||||
);
|
||||
}
|
||||
if !self.set_rag_top_k(value)? {
|
||||
self.update_app_config(|app| app.rag_top_k = value);
|
||||
}
|
||||
@@ -4125,6 +4130,12 @@ impl RequestContext {
|
||||
None => bail!("No RAG"),
|
||||
};
|
||||
|
||||
if rag.is_attached() {
|
||||
bail!(
|
||||
"Cannot edit documents on an attached RAG — Coyote does not own its source documents."
|
||||
);
|
||||
}
|
||||
|
||||
let document_paths = rag.document_paths();
|
||||
let temp_file = temp_file(&format!("-rag-{}", rag.name()), ".txt");
|
||||
tokio::fs::write(&temp_file, &document_paths.join("\n"))
|
||||
@@ -4157,7 +4168,13 @@ impl RequestContext {
|
||||
};
|
||||
self.rag_cache().invalidate(&key);
|
||||
|
||||
rag.refresh_document_paths(&new_document_paths, false, &self.app.config, abort_signal)
|
||||
rag.refresh_document_paths(
|
||||
&new_document_paths,
|
||||
false,
|
||||
false,
|
||||
&self.app.config,
|
||||
abort_signal,
|
||||
)
|
||||
.await?;
|
||||
self.rag = Some(Arc::new(rag));
|
||||
Ok(())
|
||||
@@ -4169,6 +4186,14 @@ impl RequestContext {
|
||||
None => bail!("No RAG"),
|
||||
};
|
||||
|
||||
if rag.is_attached() {
|
||||
bail!(
|
||||
"Cannot rebuild an attached RAG — Coyote does not own its source documents. \
|
||||
Re-index from the system that originally created '{}'.",
|
||||
rag.name()
|
||||
);
|
||||
}
|
||||
|
||||
let key = if self.agent.is_some() {
|
||||
RagKey::Agent(rag.name().to_string())
|
||||
} else {
|
||||
@@ -4177,7 +4202,12 @@ impl RequestContext {
|
||||
self.rag_cache().invalidate(&key);
|
||||
|
||||
let document_paths = rag.document_paths().to_vec();
|
||||
rag.refresh_document_paths(&document_paths, true, &self.app.config, abort_signal)
|
||||
println!(
|
||||
"Rebuilding re-embeds every document ({} files). \
|
||||
This will call the embedding API and may take a while.",
|
||||
rag.file_count()
|
||||
);
|
||||
rag.refresh_document_paths(&document_paths, true, true, &self.app.config, abort_signal)
|
||||
.await?;
|
||||
self.rag = Some(Arc::new(rag));
|
||||
Ok(())
|
||||
|
||||
+323
-9
@@ -132,7 +132,7 @@ impl Rag {
|
||||
let loaders = app.document_loaders.clone();
|
||||
let (spinner, spinner_rx) = Spinner::create("");
|
||||
abortable_run_with_spinner_rx(
|
||||
rag.sync_documents(doc_paths, true, loaders, Some(spinner)),
|
||||
rag.sync_documents(doc_paths, true, false, loaders, Some(spinner)),
|
||||
spinner_rx,
|
||||
abort_signal,
|
||||
)
|
||||
@@ -276,7 +276,7 @@ impl Rag {
|
||||
let loaders = app.document_loaders.clone();
|
||||
let (spinner, spinner_rx) = Spinner::create("");
|
||||
abortable_run_with_spinner_rx(
|
||||
rag.sync_documents(&paths, true, loaders, Some(spinner)),
|
||||
rag.sync_documents(&paths, true, false, loaders, Some(spinner)),
|
||||
spinner_rx,
|
||||
abort_signal,
|
||||
)
|
||||
@@ -291,6 +291,7 @@ impl Rag {
|
||||
let err = || format!("Failed to load rag '{name}' at '{}'", path.display());
|
||||
let content = fs::read_to_string(path).with_context(err)?;
|
||||
let data: RagData = serde_yaml::from_str(&content).with_context(err)?;
|
||||
data.validate().with_context(err)?;
|
||||
Self::create(app, name, path, data)
|
||||
}
|
||||
|
||||
@@ -322,13 +323,20 @@ impl Rag {
|
||||
&mut self,
|
||||
document_paths: &[String],
|
||||
refresh: bool,
|
||||
force_reingest: bool,
|
||||
app: &AppConfig,
|
||||
abort_signal: AbortSignal,
|
||||
) -> Result<()> {
|
||||
let loaders = app.document_loaders.clone();
|
||||
let (spinner, spinner_rx) = Spinner::create("");
|
||||
abortable_run_with_spinner_rx(
|
||||
self.sync_documents(document_paths, refresh, loaders, Some(spinner)),
|
||||
self.sync_documents(
|
||||
document_paths,
|
||||
refresh,
|
||||
force_reingest,
|
||||
loaders,
|
||||
Some(spinner),
|
||||
),
|
||||
spinner_rx,
|
||||
abort_signal,
|
||||
)
|
||||
@@ -455,6 +463,8 @@ impl Rag {
|
||||
.collect();
|
||||
let data = json!({
|
||||
"path": self.path,
|
||||
"driver": self.driver(),
|
||||
"attached": self.is_attached(),
|
||||
"embedding_model": self.embedding_model.id(),
|
||||
"chunk_size": self.data.chunk_size,
|
||||
"chunk_overlap": self.data.chunk_overlap,
|
||||
@@ -476,6 +486,18 @@ impl Rag {
|
||||
&self.name
|
||||
}
|
||||
|
||||
pub fn is_attached(&self) -> bool {
|
||||
self.data.attached
|
||||
}
|
||||
|
||||
pub fn driver(&self) -> &str {
|
||||
&self.data.driver
|
||||
}
|
||||
|
||||
pub fn file_count(&self) -> usize {
|
||||
self.data.files.len()
|
||||
}
|
||||
|
||||
pub fn is_temp(&self) -> bool {
|
||||
self.name == TEMP_RAG_NAME
|
||||
}
|
||||
@@ -565,9 +587,15 @@ impl Rag {
|
||||
&mut self,
|
||||
paths: &[String],
|
||||
refresh: bool,
|
||||
force_reingest: bool,
|
||||
loaders: HashMap<String, String>,
|
||||
spinner: Option<Spinner>,
|
||||
) -> Result<()> {
|
||||
debug_assert!(
|
||||
!force_reingest || refresh,
|
||||
"force_reingest requires refresh"
|
||||
);
|
||||
let refresh = refresh || force_reingest;
|
||||
if let Some(spinner) = &spinner {
|
||||
let _ = spinner.set_message(String::new());
|
||||
}
|
||||
@@ -685,11 +713,9 @@ impl Rag {
|
||||
} in loaded_documents
|
||||
{
|
||||
let hash = sha256(&contents);
|
||||
if let Some(file_ids) = to_deleted.get_mut(&hash)
|
||||
&& let Some((i, _)) = file_ids
|
||||
.iter()
|
||||
.enumerate()
|
||||
.find(|(_, v)| self.data.files[*v].path == path)
|
||||
if let Some((i, _)) =
|
||||
find_hash_skip(force_reingest, &to_deleted, &self.data.files, &hash, &path)
|
||||
&& let Some(file_ids) = to_deleted.get_mut(&hash)
|
||||
{
|
||||
if file_ids.len() == 1 {
|
||||
to_deleted.swap_remove(&hash);
|
||||
@@ -1084,16 +1110,31 @@ impl Rag {
|
||||
|
||||
#[derive(Clone, Serialize, Deserialize)]
|
||||
pub struct RagData {
|
||||
#[serde(default = "RagData::default_driver")]
|
||||
pub driver: String,
|
||||
#[serde(default)]
|
||||
pub attached: bool,
|
||||
|
||||
pub embedding_model: String,
|
||||
#[serde(default)]
|
||||
pub chunk_size: usize,
|
||||
#[serde(default)]
|
||||
pub chunk_overlap: usize,
|
||||
pub reranker_model: Option<String>,
|
||||
#[serde(default)]
|
||||
pub top_k: usize,
|
||||
pub batch_size: Option<usize>,
|
||||
#[serde(default)]
|
||||
pub next_file_id: FileId,
|
||||
#[serde(default)]
|
||||
pub document_paths: Vec<String>,
|
||||
#[serde(default)]
|
||||
pub files: IndexMap<FileId, RagFile>,
|
||||
#[serde(with = "serde_vectors")]
|
||||
#[serde(
|
||||
default,
|
||||
with = "serde_vectors",
|
||||
skip_serializing_if = "IndexMap::is_empty"
|
||||
)]
|
||||
pub vectors: IndexMap<DocumentId, Vec<f32>>,
|
||||
#[serde(default)]
|
||||
pub extractor_model: Option<String>,
|
||||
@@ -1108,6 +1149,8 @@ pub struct RagData {
|
||||
impl Debug for RagData {
|
||||
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
|
||||
f.debug_struct("RagData")
|
||||
.field("driver", &self.driver)
|
||||
.field("attached", &self.attached)
|
||||
.field("embedding_model", &self.embedding_model)
|
||||
.field("chunk_size", &self.chunk_size)
|
||||
.field("chunk_overlap", &self.chunk_overlap)
|
||||
@@ -1135,6 +1178,8 @@ impl RagData {
|
||||
graph: GraphRagConfig,
|
||||
) -> Self {
|
||||
Self {
|
||||
driver: "yaml".to_string(),
|
||||
attached: false,
|
||||
embedding_model,
|
||||
chunk_size,
|
||||
chunk_overlap,
|
||||
@@ -1152,6 +1197,52 @@ impl RagData {
|
||||
}
|
||||
}
|
||||
|
||||
fn default_driver() -> String {
|
||||
"yaml".to_string()
|
||||
}
|
||||
|
||||
pub fn validate(&self) -> Result<()> {
|
||||
if self.top_k == 0 {
|
||||
bail!(
|
||||
"top_k must be >= 1 (got 0). A top_k of 0 makes every query return \
|
||||
no results with no error. Set `top_k:` in the RAG YAML."
|
||||
);
|
||||
}
|
||||
if !self.attached {
|
||||
if self.chunk_size == 0 {
|
||||
bail!(
|
||||
"chunk_size must be >= 1 (got 0) for a non-attached RAG. A \
|
||||
chunk_size of 0 panics with a divide-by-zero while sizing \
|
||||
embedding batches. Set `chunk_size:` in the RAG YAML."
|
||||
);
|
||||
}
|
||||
if self.chunk_overlap >= self.chunk_size {
|
||||
bail!(
|
||||
"chunk_overlap ({}) must be strictly less than chunk_size ({}).",
|
||||
self.chunk_overlap,
|
||||
self.chunk_size
|
||||
);
|
||||
}
|
||||
}
|
||||
match (self.driver.as_str(), self.attached) {
|
||||
("yaml", false) => Ok(()),
|
||||
("duckdb", false) => Ok(()),
|
||||
("qdrant", true) => Ok(()),
|
||||
("qdrant", false) => Ok(()),
|
||||
("yaml", true) => bail!(
|
||||
"driver 'yaml' cannot be attached (attached: true). \
|
||||
Attached RAGs require an external driver (qdrant)."
|
||||
),
|
||||
("duckdb", true) => bail!(
|
||||
"driver 'duckdb' cannot be attached (attached: true). \
|
||||
DuckDB is a local-only driver; use 'qdrant' for external collections."
|
||||
),
|
||||
(other, _) => {
|
||||
bail!("Unknown RAG driver '{other}'. Valid drivers: yaml, duckdb, qdrant.")
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
pub fn get(&self, id: DocumentId) -> Option<&RagDocument> {
|
||||
let (file_index, document_index) = id.split();
|
||||
let file = self.files.get(&file_index)?;
|
||||
@@ -1208,6 +1299,20 @@ impl RagData {
|
||||
}
|
||||
}
|
||||
|
||||
impl Default for RagData {
|
||||
fn default() -> Self {
|
||||
RagData::new(
|
||||
String::new(),
|
||||
0,
|
||||
0,
|
||||
None,
|
||||
5,
|
||||
None,
|
||||
GraphRagConfig::default(),
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
#[derive(Debug, Clone, Serialize, Deserialize)]
|
||||
pub struct RagFile {
|
||||
hash: String,
|
||||
@@ -1451,6 +1556,30 @@ fn progress(spinner: &Option<Spinner>, message: String) {
|
||||
}
|
||||
}
|
||||
|
||||
/// Decide whether a just-loaded document may skip re-chunking and re-embedding.
|
||||
///
|
||||
/// Returns the position of the matching `FileId` within `to_deleted[hash]`, together with
|
||||
/// that `FileId`. The caller needs the position to un-mark the file for deletion. `None`
|
||||
/// means "ingest this document": either a full re-ingest was requested, or no
|
||||
/// already-indexed file has both this content hash and this path.
|
||||
fn find_hash_skip(
|
||||
force_reingest: bool,
|
||||
to_deleted: &IndexMap<String, Vec<FileId>>,
|
||||
files: &IndexMap<FileId, RagFile>,
|
||||
hash: &str,
|
||||
path: &str,
|
||||
) -> Option<(usize, FileId)> {
|
||||
if force_reingest {
|
||||
return None;
|
||||
}
|
||||
let file_ids = to_deleted.get(hash)?;
|
||||
file_ids
|
||||
.iter()
|
||||
.enumerate()
|
||||
.find(|(_, v)| files[*v].path == path)
|
||||
.map(|(i, v)| (i, *v))
|
||||
}
|
||||
|
||||
fn reciprocal_rank_fusion(
|
||||
list_of_document_ids: Vec<Vec<DocumentId>>,
|
||||
list_of_weights: Vec<f32>,
|
||||
@@ -1868,4 +1997,189 @@ mod tests {
|
||||
"higher-weight signal's top doc should rank first"
|
||||
);
|
||||
}
|
||||
|
||||
fn hash_skip_fixture() -> (IndexMap<FileId, RagFile>, IndexMap<String, Vec<FileId>>) {
|
||||
let mut files: IndexMap<FileId, RagFile> = Default::default();
|
||||
files.insert(
|
||||
7,
|
||||
RagFile {
|
||||
hash: "abc".into(),
|
||||
path: "test.txt".into(),
|
||||
documents: vec![RagDocument::new("unchanged")],
|
||||
},
|
||||
);
|
||||
let mut to_deleted: IndexMap<String, Vec<FileId>> = Default::default();
|
||||
to_deleted.insert("abc".into(), vec![7]);
|
||||
(files, to_deleted)
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn force_reingest_re_embeds_hash_identical_files() {
|
||||
let (files, to_deleted) = hash_skip_fixture();
|
||||
assert_eq!(
|
||||
find_hash_skip(true, &to_deleted, &files, "abc", "test.txt"),
|
||||
None,
|
||||
"a forced re-ingest must not skip an unchanged file"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn refresh_without_force_still_hash_skips() {
|
||||
let (files, to_deleted) = hash_skip_fixture();
|
||||
assert_eq!(
|
||||
find_hash_skip(false, &to_deleted, &files, "abc", "test.txt"),
|
||||
Some((0, 7)),
|
||||
"an unchanged file should be skipped and un-marked for deletion"
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn find_hash_skip_returns_none_on_path_change() {
|
||||
let (files, to_deleted) = hash_skip_fixture();
|
||||
assert_eq!(
|
||||
find_hash_skip(false, &to_deleted, &files, "abc", "moved.txt"),
|
||||
None
|
||||
);
|
||||
assert_eq!(
|
||||
find_hash_skip(true, &to_deleted, &files, "abc", "moved.txt"),
|
||||
None
|
||||
);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn ragdata_new_has_yaml_driver_and_not_attached() {
|
||||
let data = RagData::new(
|
||||
"text-embedding-3-small".to_string(),
|
||||
1024,
|
||||
50,
|
||||
None,
|
||||
5,
|
||||
None,
|
||||
GraphRagConfig::default(),
|
||||
);
|
||||
assert_eq!(data.driver, "yaml");
|
||||
assert!(!data.attached);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn ragdata_deserializes_without_driver_field() {
|
||||
let yaml = "
|
||||
embedding_model: text-embedding-3-small
|
||||
chunk_size: 1024
|
||||
chunk_overlap: 50
|
||||
top_k: 5
|
||||
next_file_id: 0
|
||||
document_paths: []
|
||||
files: {}
|
||||
vectors: {}
|
||||
";
|
||||
let data: RagData = serde_yaml::from_str(yaml).unwrap();
|
||||
assert_eq!(data.driver, "yaml");
|
||||
assert!(!data.attached);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn ragdata_round_trips_driver_and_attached() {
|
||||
let mut data = RagData::new(
|
||||
"text-embedding-3-small".to_string(),
|
||||
1024,
|
||||
50,
|
||||
None,
|
||||
5,
|
||||
None,
|
||||
GraphRagConfig::default(),
|
||||
);
|
||||
data.driver = "qdrant".to_string();
|
||||
data.attached = true;
|
||||
|
||||
let yaml = serde_yaml::to_string(&data).unwrap();
|
||||
let restored: RagData = serde_yaml::from_str(&yaml).unwrap();
|
||||
assert_eq!(restored.driver, "qdrant");
|
||||
assert!(restored.attached);
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn ragdata_validate_rejects_yaml_attached() {
|
||||
let mut data = RagData::new(
|
||||
"m".into(),
|
||||
1024,
|
||||
50,
|
||||
None,
|
||||
5,
|
||||
None,
|
||||
GraphRagConfig::default(),
|
||||
);
|
||||
data.attached = true;
|
||||
let err = data.validate().unwrap_err().to_string();
|
||||
assert!(err.contains("cannot be attached"), "got: {err}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn ragdata_validate_accepts_qdrant_attached() {
|
||||
let mut data = RagData::new(
|
||||
"m".into(),
|
||||
1024,
|
||||
50,
|
||||
None,
|
||||
5,
|
||||
None,
|
||||
GraphRagConfig::default(),
|
||||
);
|
||||
data.driver = "qdrant".to_string();
|
||||
data.attached = true;
|
||||
assert!(data.validate().is_ok());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn ragdata_validate_rejects_zero_top_k_from_a_truncated_yaml() {
|
||||
let yaml = "
|
||||
embedding_model: text-embedding-3-small
|
||||
chunk_size: 1024
|
||||
chunk_overlap: 50
|
||||
";
|
||||
let data: RagData = serde_yaml::from_str(yaml).unwrap();
|
||||
assert_eq!(data.top_k, 0, "a missing top_k must default to 0");
|
||||
let err = data.validate().unwrap_err().to_string();
|
||||
assert!(err.contains("top_k must be >= 1"), "got: {err}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn ragdata_validate_rejects_zero_chunk_size_when_not_attached() {
|
||||
let yaml = "
|
||||
embedding_model: text-embedding-3-small
|
||||
top_k: 5
|
||||
";
|
||||
let data: RagData = serde_yaml::from_str(yaml).unwrap();
|
||||
assert_eq!(data.chunk_size, 0);
|
||||
let err = data.validate().unwrap_err().to_string();
|
||||
assert!(err.contains("chunk_size must be >= 1"), "got: {err}");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn ragdata_validate_allows_zero_chunk_size_when_attached() {
|
||||
let yaml = "
|
||||
driver: qdrant
|
||||
attached: true
|
||||
embedding_model: text-embedding-3-small
|
||||
top_k: 5
|
||||
";
|
||||
let data: RagData = serde_yaml::from_str(yaml).unwrap();
|
||||
assert_eq!(data.chunk_size, 0);
|
||||
assert!(data.validate().is_ok());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn ragdata_validate_rejects_overlap_not_less_than_chunk_size() {
|
||||
let data = RagData::new(
|
||||
"m".into(),
|
||||
100,
|
||||
100,
|
||||
None,
|
||||
5,
|
||||
None,
|
||||
GraphRagConfig::default(),
|
||||
);
|
||||
let err = data.validate().unwrap_err().to_string();
|
||||
assert!(err.contains("chunk_overlap"), "got: {err}");
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user