diff --git a/CHANGELOG.md b/CHANGELOG.md index 7796fdf6..14cca6ba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,8 @@ All notable changes to ConceptWeave are documented here. ### Fixed +- Research change plans reject changed supporting evidence and incomplete inventories before approval, and preserve the identity of the reviewed evidence. + - Duplicate review rejects incomplete source inventories and changed supporting evidence before approval, while retaining reversible identity mappings. - Research evaluation rejects incomplete source inventories and invalidates prior approvals when retained source metadata changes. - Research reports retain standalone files and notes that previously disappeared from the classification view, and flag sources whose parent relationships remain unresolved. diff --git a/crates/conceptweave-zotero/src/lib.rs b/crates/conceptweave-zotero/src/lib.rs index 67c81568..7a2ad600 100644 --- a/crates/conceptweave-zotero/src/lib.rs +++ b/crates/conceptweave-zotero/src/lib.rs @@ -96,10 +96,13 @@ pub struct ItemData { } /// A Zotero item tag. -#[derive(Debug, Clone, Deserialize, Serialize)] +#[derive(Debug, Clone, PartialEq, Eq, PartialOrd, Ord, Deserialize, Serialize)] pub struct ItemTag { /// Tag text. pub tag: String, + /// Zotero automatic-tag marker, preserved for exact write rollback. + #[serde(default, rename = "type", skip_serializing_if = "Option::is_none")] + pub tag_type: Option, } /// One mutually exclusive proposed disposition. @@ -161,7 +164,7 @@ pub struct ClassifiedItem { /// Collection keys observed with the item. pub collection_keys: Vec, /// Tag text observed with the item. - pub tags: Vec, + pub tags: Vec, /// Proposed disposition; never an authoritative governance decision. pub proposed_disposition: Disposition, /// Deterministic reason for abstention, absent when a rule proposes a disposition. @@ -291,6 +294,158 @@ impl fmt::Display for DuplicateReviewError { impl std::error::Error for DuplicateReviewError {} +/// Requested behavior for a reviewed classification change set. +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq, Serialize)] +#[serde(rename_all = "snake_case")] +pub enum WriteMode { + /// Validate and emit a plan without contacting Zotero. + #[default] + DryRun, + /// Permit a future authenticated Zotero 10+ adapter to apply the plan. + Execute, +} + +/// One steward-reviewed complete collection and tag replacement. +#[derive(Debug, Clone, PartialEq, Eq, Deserialize, Serialize)] +pub struct ReviewedClassificationChange { + /// Stable top-level bibliographic item key. + pub item_key: String, + /// Exact item revision reviewed by the steward. + pub item_version: u64, + /// Approved classification; abstention cannot be written. + pub reviewed_disposition: Disposition, + /// Complete collection state observed before the change. + pub before_collection_keys: Vec, + /// Complete collection state requested after the change. + pub after_collection_keys: Vec, + /// Complete tag state observed before the change. + pub before_tags: Vec, + /// Complete tag state requested after the change. + pub after_tags: Vec, +} + +/// Governance-bound reviewed changes retained outside the repository. +#[derive(Debug, Clone, PartialEq, Eq, Deserialize, Serialize)] +pub struct ReviewedClassificationWriteSet { + /// Opaque review receipt identifier. + pub review_id: String, + /// Opaque governance authority receipt. + pub authority_receipt: String, + /// Exact Local API server identity, when supplied by Zotero. + pub server_id: Option, + /// Exact Zotero version reviewed for write capability. + pub zotero_version: String, + /// Exact reviewed library revision. + pub library_version: u64, + /// Exact reviewed classifier revision. + pub rule_revision: String, + /// Exact reviewed raw-snapshot digest. + pub snapshot_digest: String, + /// Required identity of reviewed proposals, unclassified metadata, and pending keys. + pub proposal_digest: String, + /// Exact item-key/item-version coordinates reviewed by the steward. + pub snapshot_items: Vec, + /// Reviewed item-level changes. + pub changes: Vec, +} + +/// One deterministic item operation with an exact rollback state. +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +pub struct ClassificationWriteOperation { + /// Stable top-level bibliographic item key. + pub item_key: String, + /// Optimistic item-version precondition. + pub item_version: u64, + /// Approved classification. + pub reviewed_disposition: Disposition, + /// Complete collection state before the change. + pub before_collection_keys: Vec, + /// Complete collection state after the change. + pub after_collection_keys: Vec, + /// Complete collection rollback state. + pub rollback_collection_keys: Vec, + /// Complete tag state before the change. + pub before_tags: Vec, + /// Complete tag state after the change. + pub after_tags: Vec, + /// Complete tag rollback state, including Zotero tag type. + pub rollback_tags: Vec, +} + +/// Local-only, snapshot-bound plan for reviewed Zotero classification writes. +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +pub struct ClassificationWritePlan { + /// Requested write behavior; dry-run is the default. + pub mode: WriteMode, + /// Opaque review receipt identifier. + pub review_id: String, + /// Opaque governance authority receipt. + pub authority_receipt: String, + /// Exact Local API server identity. + pub server_id: Option, + /// Exact Zotero version used to establish execute eligibility. + pub zotero_version: String, + /// Exact library-version precondition. + pub library_version: u64, + /// Exact classifier revision. + pub rule_revision: String, + /// Exact raw-snapshot digest. + pub snapshot_digest: String, + /// Content identity retained from the independently verified review. + pub proposal_digest: String, + /// Deterministically ordered item operations. + pub operations: Vec, + /// Classification writes never delete source records or attachments. + pub source_records_preserved: bool, +} + +/// A fail-closed reviewed write-plan contract violation. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum WritePlanError { + /// Required review metadata or item changes are absent. + InvalidReview, + /// The reviewed set belongs to another snapshot. + SnapshotMismatch, + /// The caller's governance boundary rejected the complete reviewed set. + UnverifiedApproval, + /// A change does not identify a classified top-level item. + UnknownItem, + /// More than one change targets the same item. + DuplicateItem, + /// An item revision or complete before-state is stale. + StaleItem, + /// A collection or tag value is blank or duplicated. + InvalidMetadata, + /// A reviewed change makes no collection or tag change. + NoChange, + /// An abstention cannot become a write instruction. + UnreviewedDisposition, + /// Execute mode is unsupported by this Zotero major version. + UnsupportedExecute, + /// Execute mode lacks a nonblank Local API server identity. + MissingServerIdentity, +} + +impl fmt::Display for WritePlanError { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + formatter.write_str(match self { + Self::InvalidReview => "classification write review is invalid", + Self::SnapshotMismatch => "classification write review does not match the snapshot", + Self::UnverifiedApproval => "classification write approval is unverified", + Self::UnknownItem => "classification write targets an unknown item", + Self::DuplicateItem => "classification write repeats an item", + Self::StaleItem => "classification write item preconditions are stale", + Self::InvalidMetadata => "classification write metadata is blank or duplicated", + Self::NoChange => "classification write contains no metadata change", + Self::UnreviewedDisposition => "steward-review abstention cannot be written", + Self::UnsupportedExecute => "this Zotero version does not support local execution", + Self::MissingServerIdentity => "local execution requires a Zotero server identity", + }) + } +} + +impl std::error::Error for WritePlanError {} + /// Complete local classification report for one immutable library version. #[derive(Debug, Serialize)] pub struct ClassificationReport { @@ -843,6 +998,189 @@ where }) } +fn normalized_metadata( + collection_keys: &[String], + tags: &[ItemTag], +) -> Result<(Vec, Vec), WritePlanError> { + if collection_keys.iter().any(|key| key.trim().is_empty()) + || tags.iter().any(|tag| tag.tag.trim().is_empty()) + || tags + .iter() + .any(|tag| tag.tag_type.is_some_and(|tag_type| tag_type > 1)) + || collection_keys.iter().collect::>().len() != collection_keys.len() + || tags + .iter() + .map(|tag| tag.tag.as_str()) + .collect::>() + .len() + != tags.len() + { + return Err(WritePlanError::InvalidMetadata); + } + let mut collection_keys = collection_keys.to_vec(); + let mut tags = tags.to_vec(); + for tag in &mut tags { + if tag.tag_type == Some(0) { + tag.tag_type = None; + } + } + collection_keys.sort(); + tags.sort(); + Ok((collection_keys, tags)) +} + +/// Builds a deterministic reviewed write plan without contacting or mutating Zotero. +/// +/// Local identity, execution-mode, and metadata checks finish before the caller's +/// governance verifier runs. Invalid input never consumes a one-use approval; +/// a locally valid complete review is verified exactly once before returning a plan. +/// The verifier must authenticate every field of the reviewed set, including its +/// proposal digest and requested changes. Recomputing a digest grants no authority. +pub fn build_classification_write_plan( + report: &ClassificationReport, + reviewed: &ReviewedClassificationWriteSet, + mode: WriteMode, + verify_review: F, +) -> Result +where + F: FnOnce(&ReviewedClassificationWriteSet) -> bool, +{ + if reviewed.review_id.trim().is_empty() + || reviewed.authority_receipt.trim().is_empty() + || reviewed.rule_revision.trim().is_empty() + || reviewed.snapshot_digest.trim().is_empty() + || reviewed.proposal_digest.trim().is_empty() + || reviewed.changes.is_empty() + { + return Err(WritePlanError::InvalidReview); + } + if reviewed.server_id != report.server_id + || reviewed.zotero_version != report.zotero_version + || reviewed.library_version != report.library_version + || reviewed.rule_revision != report.rule_revision + || reviewed.snapshot_digest != report.snapshot_digest + || reviewed.snapshot_items != report.snapshot_items + { + return Err(WritePlanError::SnapshotMismatch); + } + if mode == WriteMode::Execute + && report + .zotero_version + .split('.') + .next() + .and_then(|major| major.parse::().ok()) + .is_none_or(|major| major < 10) + { + return Err(WritePlanError::UnsupportedExecute); + } + if mode == WriteMode::Execute + && reviewed + .server_id + .as_deref() + .is_none_or(|server_id| server_id.trim().is_empty()) + { + return Err(WritePlanError::MissingServerIdentity); + } + + if report + .snapshot_items + .iter() + .any(|item| item.item_key.trim().is_empty()) + || report + .snapshot_items + .iter() + .map(|item| item.item_key.as_str()) + .collect::>() + .len() + != report.snapshot_items.len() + || report + .classified_items + .iter() + .map(|item| item.item_key.as_str()) + .collect::>() + .len() + != report.classified_items.len() + { + return Err(WritePlanError::InvalidReview); + } + let classified = report + .classified_items + .iter() + .map(|item| (item.item_key.as_str(), item)) + .collect::>(); + let snapshot_revisions = report + .snapshot_items + .iter() + .map(|item| (item.item_key.as_str(), item.item_version)) + .collect::>(); + let mut seen_items = BTreeSet::new(); + let mut operations = Vec::with_capacity(reviewed.changes.len()); + for change in &reviewed.changes { + if change.item_key.trim().is_empty() { + return Err(WritePlanError::InvalidReview); + } + if !seen_items.insert(change.item_key.as_str()) { + return Err(WritePlanError::DuplicateItem); + } + if change.reviewed_disposition == Disposition::NeedsStewardReview { + return Err(WritePlanError::UnreviewedDisposition); + } + let item = classified + .get(change.item_key.as_str()) + .ok_or(WritePlanError::UnknownItem)?; + if snapshot_revisions.get(change.item_key.as_str()) != Some(&item.item_version) { + return Err(WritePlanError::StaleItem); + } + let (actual_collections, actual_tags) = + normalized_metadata(&item.collection_keys, &item.tags)?; + let (before_collections, before_tags) = + normalized_metadata(&change.before_collection_keys, &change.before_tags)?; + let (after_collections, after_tags) = + normalized_metadata(&change.after_collection_keys, &change.after_tags)?; + if change.item_version != item.item_version + || before_collections != actual_collections + || before_tags != actual_tags + { + return Err(WritePlanError::StaleItem); + } + if before_collections == after_collections && before_tags == after_tags { + return Err(WritePlanError::NoChange); + } + operations.push(ClassificationWriteOperation { + item_key: change.item_key.clone(), + item_version: change.item_version, + reviewed_disposition: change.reviewed_disposition, + rollback_collection_keys: before_collections.clone(), + before_collection_keys: before_collections, + after_collection_keys: after_collections, + rollback_tags: before_tags.clone(), + before_tags, + after_tags, + }); + } + operations.sort_by(|left, right| left.item_key.cmp(&right.item_key)); + validate_classification_report(report).map_err(|_| WritePlanError::InvalidReview)?; + if reviewed.proposal_digest != classification_proposal_digest(report) { + return Err(WritePlanError::SnapshotMismatch); + } + if !verify_review(reviewed) { + return Err(WritePlanError::UnverifiedApproval); + } + Ok(ClassificationWritePlan { + mode, + review_id: reviewed.review_id.clone(), + authority_receipt: reviewed.authority_receipt.clone(), + server_id: reviewed.server_id.clone(), + zotero_version: reviewed.zotero_version.clone(), + library_version: reviewed.library_version, + rule_revision: reviewed.rule_revision.clone(), + snapshot_digest: reviewed.snapshot_digest.clone(), + proposal_digest: reviewed.proposal_digest.clone(), + operations, + source_records_preserved: true, + }) +} + /// Failure raised when a bounded, immutable Local API read cannot be proven. #[derive(Debug)] pub enum ReadError { @@ -1411,7 +1749,7 @@ fn classify_item(item: &ZoteroItem, child_item_keys: Vec) -> ClassifiedI item_type: item.data.item_type.clone(), title: item.data.title.clone(), collection_keys: item.data.collections.clone(), - tags: item.data.tags.iter().map(|tag| tag.tag.clone()).collect(), + tags: item.data.tags.clone(), proposed_disposition, abstention_reason, evidence: ClassificationEvidence { @@ -1867,6 +2205,7 @@ mod tests { standalone.data.collections = vec!["COLLECTION".into()]; standalone.data.tags = vec![ItemTag { tag: "Evidence".into(), + tag_type: None, }]; let report = read_snapshot_with(&mut |_| { Ok(fetched_page( @@ -1974,6 +2313,7 @@ mod tests { let mut generation = item("B", "journalArticle", "Ontology Learning", "10.1/X", ""); generation.data.tags.push(ItemTag { tag: "SHACL".into(), + tag_type: Some(1), }); let report = classify_snapshot( "9.0.6".into(), diff --git a/crates/conceptweave-zotero/tests/classification_write_plan.rs b/crates/conceptweave-zotero/tests/classification_write_plan.rs new file mode 100644 index 00000000..567c8b40 --- /dev/null +++ b/crates/conceptweave-zotero/tests/classification_write_plan.rs @@ -0,0 +1,691 @@ +use conceptweave_zotero::{ + Disposition, ItemData, ItemTag, ReviewedClassificationChange, ReviewedClassificationWriteSet, + WriteMode, WritePlanError, ZoteroItem, build_classification_write_plan, + classification_proposal_digest, classify_snapshot, +}; + +#[test] +fn write_scope_rejects_changed_evidence_before_authority() { + let mut report = classification_report("10.0.1"); + let review = reviewed(&report); + report.classified_items[0] + .title + .push_str(" changed evidence"); + let called = std::cell::Cell::new(false); + let result = build_classification_write_plan(&report, &review, WriteMode::DryRun, |_| { + called.set(true); + true + }); + assert_eq!(result, Err(WritePlanError::SnapshotMismatch)); + assert!(!called.get()); +} + +#[test] +fn write_scope_rejects_inconsistent_inventory_before_authority() { + for mutation in ["count", "pending", "audit"] { + let mut report = classification_report("10.0.1"); + match mutation { + "count" => report.observed_item_count += 1, + "pending" => report.pending_source_item_keys.push("absent".into()), + "audit" => report.audit_summary.failure_count += 1, + _ => unreachable!(), + } + let review = reviewed(&report); + let called = std::cell::Cell::new(false); + let result = build_classification_write_plan(&report, &review, WriteMode::DryRun, |_| { + called.set(true); + true + }); + assert_eq!(result, Err(WritePlanError::InvalidReview), "{mutation}"); + assert!(!called.get(), "{mutation}"); + } +} + +#[test] +fn write_scope_requires_binding_and_independent_approval() { + let mut report = classification_report("10.0.1"); + let approved = reviewed(&report); + let mut serialized = serde_json::to_value(&approved).unwrap(); + serialized + .as_object_mut() + .unwrap() + .remove("proposal_digest"); + assert!(serde_json::from_value::(serialized).is_err()); + + let mut review = approved.clone(); + review.proposal_digest = " ".into(); + assert_eq!( + build_classification_write_plan(&report, &review, WriteMode::DryRun, |_| { + panic!("blank binding must not reach authority") + }), + Err(WritePlanError::InvalidReview) + ); + let plan = build_classification_write_plan(&report, &approved, WriteMode::DryRun, |set| { + set == &approved + }) + .unwrap(); + assert_eq!(plan.proposal_digest, approved.proposal_digest); + + report.classified_items[0] + .title + .push_str(" changed evidence"); + review = approved.clone(); + review.proposal_digest = classification_proposal_digest(&report); + let calls = std::cell::Cell::new(0); + assert_eq!( + build_classification_write_plan(&report, &review, WriteMode::DryRun, |set| { + calls.set(calls.get() + 1); + set == &approved + }), + Err(WritePlanError::UnverifiedApproval) + ); + assert_eq!(calls.get(), 1); +} + +fn tag(name: &str, tag_type: Option) -> ItemTag { + ItemTag { + tag: name.into(), + tag_type, + } +} + +fn classification_report(version: &str) -> conceptweave_zotero::ClassificationReport { + classify_snapshot( + version.into(), + Some("server-1".into()), + 42, + vec![ + ZoteroItem { + source_record: None, + key: "B".into(), + version: 9, + data: ItemData { + item_type: "book".into(), + title: "ontology evaluation".into(), + abstract_note: String::new(), + doi: String::new(), + parent_item: String::new(), + collections: vec!["source_collection".into()], + tags: vec![tag("Imported", Some(1))], + }, + }, + ZoteroItem { + source_record: None, + key: "A".into(), + version: 7, + data: ItemData { + item_type: "journalArticle".into(), + title: "ontology learning".into(), + abstract_note: String::new(), + doi: String::new(), + parent_item: String::new(), + collections: vec![], + tags: vec![], + }, + }, + ], + ) +} + +fn reviewed(report: &conceptweave_zotero::ClassificationReport) -> ReviewedClassificationWriteSet { + ReviewedClassificationWriteSet { + review_id: "review-1".into(), + authority_receipt: "authority-1".into(), + server_id: report.server_id.clone(), + zotero_version: report.zotero_version.clone(), + library_version: report.library_version, + rule_revision: report.rule_revision.into(), + snapshot_digest: report.snapshot_digest.clone(), + proposal_digest: classification_proposal_digest(report), + snapshot_items: report.snapshot_items.clone(), + changes: vec![ + ReviewedClassificationChange { + item_key: "B".into(), + item_version: 9, + reviewed_disposition: Disposition::EvaluationGovernance, + before_collection_keys: vec!["source_collection".into()], + after_collection_keys: vec!["evaluation_collection".into()], + before_tags: vec![tag("Imported", Some(1))], + after_tags: vec![tag("Evaluation", None), tag("Imported", Some(1))], + }, + ReviewedClassificationChange { + item_key: "A".into(), + item_version: 7, + reviewed_disposition: Disposition::Generation, + before_collection_keys: vec![], + after_collection_keys: vec!["generation_collection".into()], + before_tags: vec![], + after_tags: vec![tag("Generation", None)], + }, + ], + } +} + +#[test] +fn dry_run_is_default_and_preserves_exact_rollback_state() { + assert_eq!(WriteMode::default(), WriteMode::DryRun); + let report = classification_report("9.0.6"); + let plan = + build_classification_write_plan(&report, &reviewed(&report), WriteMode::default(), |_| { + true + }) + .expect("reviewed dry-run changes must produce a plan"); + + assert_eq!(plan.mode, WriteMode::DryRun); + assert_eq!(plan.operations[0].item_key, "A"); + assert_eq!(plan.operations[1].item_key, "B"); + assert_eq!( + plan.operations[1].rollback_collection_keys, + plan.operations[1].before_collection_keys + ); + assert_eq!( + plan.operations[1].rollback_tags, + vec![tag("Imported", Some(1))] + ); + assert!(plan.source_records_preserved); +} + +#[test] +fn write_plan_fails_closed_for_untrusted_stale_or_unsafe_changes() { + let report = classification_report("9.0.6"); + assert_eq!( + build_classification_write_plan(&report, &reviewed(&report), WriteMode::Execute, |_| true), + Err(WritePlanError::UnsupportedExecute) + ); + assert_eq!( + build_classification_write_plan(&report, &reviewed(&report), WriteMode::DryRun, |_| false), + Err(WritePlanError::UnverifiedApproval) + ); + + let mut review = reviewed(&report); + review.snapshot_digest = "sha256:stale".into(); + assert_eq!( + build_classification_write_plan(&report, &review, WriteMode::DryRun, |_| true), + Err(WritePlanError::SnapshotMismatch) + ); + review = reviewed(&report); + review.changes[0].item_version += 1; + assert_eq!( + build_classification_write_plan(&report, &review, WriteMode::DryRun, |_| true), + Err(WritePlanError::StaleItem) + ); + review = reviewed(&report); + review.changes[0].before_tags.clear(); + assert_eq!( + build_classification_write_plan(&report, &review, WriteMode::DryRun, |_| true), + Err(WritePlanError::StaleItem) + ); + review = reviewed(&report); + review.changes[0].after_tags = review.changes[0].before_tags.clone(); + review.changes[0].after_collection_keys = review.changes[0].before_collection_keys.clone(); + assert_eq!( + build_classification_write_plan(&report, &review, WriteMode::DryRun, |_| true), + Err(WritePlanError::NoChange) + ); + review = reviewed(&report); + review.changes[0].reviewed_disposition = Disposition::NeedsStewardReview; + assert_eq!( + build_classification_write_plan(&report, &review, WriteMode::DryRun, |_| true), + Err(WritePlanError::UnreviewedDisposition) + ); + review = reviewed(&report); + review.changes[1] = review.changes[0].clone(); + assert_eq!( + build_classification_write_plan(&report, &review, WriteMode::DryRun, |_| true), + Err(WritePlanError::DuplicateItem) + ); + review = reviewed(&report); + review.changes[0].item_key = "missing".into(); + assert_eq!( + build_classification_write_plan(&report, &review, WriteMode::DryRun, |_| true), + Err(WritePlanError::UnknownItem) + ); + review = reviewed(&report); + review.changes[0].after_tags.push(tag(" ", None)); + assert_eq!( + build_classification_write_plan(&report, &review, WriteMode::DryRun, |_| true), + Err(WritePlanError::InvalidMetadata) + ); + + for invalidate in [ + |review: &mut ReviewedClassificationWriteSet| review.review_id.clear(), + |review: &mut ReviewedClassificationWriteSet| review.authority_receipt.clear(), + |review: &mut ReviewedClassificationWriteSet| review.rule_revision.clear(), + |review: &mut ReviewedClassificationWriteSet| review.snapshot_digest.clear(), + |review: &mut ReviewedClassificationWriteSet| review.changes.clear(), + ] { + review = reviewed(&report); + invalidate(&mut review); + assert_eq!( + build_classification_write_plan(&report, &review, WriteMode::DryRun, |_| true), + Err(WritePlanError::InvalidReview) + ); + } + for make_stale in [ + |review: &mut ReviewedClassificationWriteSet| review.server_id = Some("other".into()), + |review: &mut ReviewedClassificationWriteSet| review.library_version += 1, + |review: &mut ReviewedClassificationWriteSet| review.rule_revision = "other".into(), + |review: &mut ReviewedClassificationWriteSet| { + review.snapshot_digest = "sha256:other".into() + }, + ] { + review = reviewed(&report); + make_stale(&mut review); + assert_eq!( + build_classification_write_plan(&report, &review, WriteMode::DryRun, |_| true), + Err(WritePlanError::SnapshotMismatch) + ); + } + review = reviewed(&report); + review.changes[0].item_key.clear(); + assert_eq!( + build_classification_write_plan(&report, &review, WriteMode::DryRun, |_| true), + Err(WritePlanError::InvalidReview) + ); + review = reviewed(&report); + review.changes[0].before_collection_keys.clear(); + assert_eq!( + build_classification_write_plan(&report, &review, WriteMode::DryRun, |_| true), + Err(WritePlanError::StaleItem) + ); + for invalidate_metadata in [ + |change: &mut ReviewedClassificationChange| change.after_collection_keys.push(" ".into()), + |change: &mut ReviewedClassificationChange| { + change + .after_collection_keys + .push(change.after_collection_keys[0].clone()) + }, + |change: &mut ReviewedClassificationChange| { + change.after_tags.push(change.after_tags[0].clone()) + }, + ] { + review = reviewed(&report); + invalidate_metadata(&mut review.changes[0]); + assert_eq!( + build_classification_write_plan(&report, &review, WriteMode::DryRun, |_| true), + Err(WritePlanError::InvalidMetadata) + ); + } + + review = reviewed(&report); + review.changes[0].before_tags.push(tag("Imported", None)); + assert_eq!( + build_classification_write_plan(&report, &review, WriteMode::DryRun, |_| true), + Err(WritePlanError::InvalidMetadata) + ); + let mut invalid_report = classification_report("9.0.6"); + invalid_report.classified_items[0].tags.push(tag(" ", None)); + assert_eq!( + build_classification_write_plan( + &invalid_report, + &reviewed(&invalid_report), + WriteMode::DryRun, + |_| true + ), + Err(WritePlanError::InvalidMetadata) + ); + + review = reviewed(&report); + review.changes[0].after_collection_keys = review.changes[0].before_collection_keys.clone(); + assert!(build_classification_write_plan(&report, &review, WriteMode::DryRun, |_| true).is_ok()); + + let version_ten = classification_report("10.0.0"); + assert!( + build_classification_write_plan( + &version_ten, + &reviewed(&version_ten), + WriteMode::Execute, + |_| true + ) + .is_ok() + ); + let no_server = classify_snapshot( + "10.0.0".into(), + None, + 42, + vec![ZoteroItem { + source_record: None, + key: "A".into(), + version: 7, + data: ItemData { + item_type: "journalArticle".into(), + title: "ontology learning".into(), + abstract_note: String::new(), + doi: String::new(), + parent_item: String::new(), + collections: vec![], + tags: vec![], + }, + }], + ); + let mut no_server_review = ReviewedClassificationWriteSet { + review_id: "review".into(), + authority_receipt: "authority".into(), + server_id: None, + zotero_version: no_server.zotero_version.clone(), + library_version: no_server.library_version, + rule_revision: no_server.rule_revision.into(), + snapshot_digest: no_server.snapshot_digest.clone(), + proposal_digest: classification_proposal_digest(&no_server), + snapshot_items: no_server.snapshot_items.clone(), + changes: vec![ReviewedClassificationChange { + item_key: "A".into(), + item_version: 7, + reviewed_disposition: Disposition::Generation, + before_collection_keys: vec![], + after_collection_keys: vec!["generation_collection".into()], + before_tags: vec![], + after_tags: vec![], + }], + }; + assert_eq!( + build_classification_write_plan(&no_server, &no_server_review, WriteMode::Execute, |_| { + true + }), + Err(WritePlanError::MissingServerIdentity) + ); + no_server_review.server_id = Some(" ".into()); + let mut blank_server_report = no_server; + blank_server_report.server_id = Some(" ".into()); + assert_eq!( + build_classification_write_plan( + &blank_server_report, + &no_server_review, + WriteMode::Execute, + |_| true + ), + Err(WritePlanError::MissingServerIdentity) + ); + + let mut duplicate_report = classification_report("10.0.0"); + duplicate_report.classified_items[1].item_key = "A".into(); + assert_eq!( + build_classification_write_plan( + &duplicate_report, + &reviewed(&duplicate_report), + WriteMode::DryRun, + |_| true + ), + Err(WritePlanError::InvalidReview) + ); + let malformed_version = classification_report("unknown"); + assert_eq!( + build_classification_write_plan( + &malformed_version, + &reviewed(&malformed_version), + WriteMode::Execute, + |_| true + ), + Err(WritePlanError::UnsupportedExecute) + ); + + let mut changed_version = classification_report("9.0.6"); + let changed_version_review = reviewed(&changed_version); + changed_version.zotero_version = "10.0.0".into(); + assert_eq!( + build_classification_write_plan( + &changed_version, + &changed_version_review, + WriteMode::Execute, + |_| true + ), + Err(WritePlanError::SnapshotMismatch) + ); + + let exact_report = classification_report("10.0.0"); + let mut changed_snapshot_review = reviewed(&exact_report); + changed_snapshot_review.snapshot_items[0].item_version += 1; + assert_eq!( + build_classification_write_plan( + &exact_report, + &changed_snapshot_review, + WriteMode::DryRun, + |_| true + ), + Err(WritePlanError::SnapshotMismatch) + ); + + let mut blank_snapshot_key = classification_report("10.0.0"); + blank_snapshot_key.snapshot_items[0].item_key = " ".into(); + assert_eq!( + build_classification_write_plan( + &blank_snapshot_key, + &reviewed(&blank_snapshot_key), + WriteMode::DryRun, + |_| true + ), + Err(WritePlanError::InvalidReview) + ); + + let mut duplicate_snapshot = classification_report("10.0.0"); + duplicate_snapshot + .snapshot_items + .push(duplicate_snapshot.snapshot_items[0].clone()); + assert_eq!( + build_classification_write_plan( + &duplicate_snapshot, + &reviewed(&duplicate_snapshot), + WriteMode::DryRun, + |_| true + ), + Err(WritePlanError::InvalidReview) + ); + + let mut detached_item = classification_report("10.0.0"); + detached_item.classified_items[0].item_version += 1; + assert_eq!( + build_classification_write_plan( + &detached_item, + &reviewed(&detached_item), + WriteMode::DryRun, + |_| true + ), + Err(WritePlanError::StaleItem) + ); + + let mut manual_marker = reviewed(&version_ten); + manual_marker.changes[0].after_tags[0].tag_type = Some(0); + let manual_plan = + build_classification_write_plan(&version_ten, &manual_marker, WriteMode::DryRun, |_| true) + .unwrap(); + assert_eq!(manual_plan.operations[1].after_tags[0].tag_type, None); + + manual_marker.changes[0].after_tags[0].tag_type = Some(2); + assert_eq!( + build_classification_write_plan(&version_ten, &manual_marker, WriteMode::DryRun, |_| true), + Err(WritePlanError::InvalidMetadata) + ); + + for (error, fragment) in [ + (WritePlanError::InvalidReview, "invalid"), + (WritePlanError::SnapshotMismatch, "snapshot"), + (WritePlanError::UnverifiedApproval, "unverified"), + (WritePlanError::UnknownItem, "unknown"), + (WritePlanError::DuplicateItem, "repeats"), + (WritePlanError::StaleItem, "stale"), + (WritePlanError::InvalidMetadata, "duplicated"), + (WritePlanError::NoChange, "no metadata"), + (WritePlanError::UnreviewedDisposition, "cannot be written"), + (WritePlanError::UnsupportedExecute, "does not support"), + (WritePlanError::MissingServerIdentity, "server identity"), + ] { + assert!(error.to_string().contains(fragment)); + } +} + +/// A governance verifier may consume a one-use receipt; local rejection must not call it. +fn assert_rejected_before_verification( + report: &conceptweave_zotero::ClassificationReport, + review: &ReviewedClassificationWriteSet, + mode: WriteMode, + expected: WritePlanError, +) { + for verifier_result in [true, false] { + let calls = std::cell::Cell::new(0); + let result = build_classification_write_plan(report, review, mode, |_| { + calls.set(calls.get() + 1); + verifier_result + }); + assert_eq!(calls.get(), 0, "local {expected:?} must preserve approval"); + assert_eq!(result, Err(expected)); + } +} + +#[test] +fn write_verifier_waits_for_every_item_change_to_be_valid() { + let report = classification_report("10.0.0"); + for expected in [ + WritePlanError::UnknownItem, + WritePlanError::StaleItem, + WritePlanError::DuplicateItem, + WritePlanError::UnreviewedDisposition, + WritePlanError::NoChange, + WritePlanError::InvalidReview, + ] { + let mut review = reviewed(&report); + // Reject the second operation after the first has passed local validation. + match expected { + WritePlanError::UnknownItem => review.changes[1].item_key = "missing".into(), + WritePlanError::StaleItem => review.changes[1].item_version += 1, + WritePlanError::DuplicateItem => review.changes[1] = review.changes[0].clone(), + WritePlanError::UnreviewedDisposition => { + review.changes[1].reviewed_disposition = Disposition::NeedsStewardReview; + } + WritePlanError::NoChange => { + review.changes[1].after_collection_keys = + review.changes[1].before_collection_keys.clone(); + review.changes[1].after_tags = review.changes[1].before_tags.clone(); + } + WritePlanError::InvalidReview => review.changes[1].item_key.clear(), + _ => unreachable!("only enumerated local item failures are tested"), + } + assert_rejected_before_verification(&report, &review, WriteMode::DryRun, expected); + } +} + +#[test] +fn write_verifier_waits_for_metadata_normalization_and_before_state_checks() { + for location in [ + "actual", + "before", + "after", + "tag_type", + "duplicate", + "stale", + ] { + let mut report = classification_report("10.0.0"); + let mut review = reviewed(&report); + let mut expected = WritePlanError::InvalidMetadata; + match location { + "actual" => report.classified_items[0].tags.push(tag(" ", None)), + "before" => review.changes[1].before_collection_keys.push(" ".into()), + "after" => review.changes[1].after_tags.push(tag(" ", None)), + "tag_type" => review.changes[1].after_tags[0].tag_type = Some(2), + "duplicate" => { + let collection = review.changes[1].after_collection_keys[0].clone(); + review.changes[1].after_collection_keys.push(collection); + } + "stale" => { + review.changes[1].before_tags.push(tag("unobserved", None)); + expected = WritePlanError::StaleItem; + } + _ => unreachable!("only enumerated metadata locations are tested"), + } + assert_rejected_before_verification(&report, &review, WriteMode::DryRun, expected); + } +} + +#[test] +fn write_verifier_waits_for_report_membership_and_review_identity_checks() { + for location in ["blank", "duplicate", "classified", "detached"] { + let mut report = classification_report("10.0.0"); + let mut expected = WritePlanError::InvalidReview; + match location { + "blank" => report.snapshot_items[0].item_key.clear(), + "duplicate" => report.snapshot_items.push(report.snapshot_items[0].clone()), + "classified" => report.classified_items[1].item_key = "A".into(), + "detached" => { + report.classified_items[0].item_version += 1; + expected = WritePlanError::StaleItem; + } + _ => unreachable!("only enumerated report failures are tested"), + } + let review = reviewed(&report); + assert_rejected_before_verification(&report, &review, WriteMode::DryRun, expected); + } + let report = classification_report("10.0.0"); + let mut review = reviewed(&report); + review.review_id.clear(); + assert_rejected_before_verification( + &report, + &review, + WriteMode::DryRun, + WritePlanError::InvalidReview, + ); + let mut review = reviewed(&report); + review.library_version += 1; + assert_rejected_before_verification( + &report, + &review, + WriteMode::DryRun, + WritePlanError::SnapshotMismatch, + ); +} + +#[test] +fn write_verifier_waits_for_supported_execute_controls() { + for version in ["9.0.6", "unknown"] { + let report = classification_report(version); + assert_rejected_before_verification( + &report, + &reviewed(&report), + WriteMode::Execute, + WritePlanError::UnsupportedExecute, + ); + } + for server_id in [None, Some(" ".to_owned())] { + let mut report = classification_report("10.0.0"); + report.server_id = server_id; + assert_rejected_before_verification( + &report, + &reviewed(&report), + WriteMode::Execute, + WritePlanError::MissingServerIdentity, + ); + } +} + +#[test] +fn write_verifier_receives_valid_complete_review_once_and_preserves_plan_behavior() { + let report = classification_report("10.0.0"); + let review = reviewed(&report); + for mode in [WriteMode::DryRun, WriteMode::Execute] { + for verified in [false, true] { + let calls = std::cell::Cell::new(0); + let result = build_classification_write_plan(&report, &review, mode, |received| { + calls.set(calls.get() + 1); + assert_eq!(received, &review); + verified + }); + assert_eq!(calls.get(), 1); + if !verified { + assert_eq!(result, Err(WritePlanError::UnverifiedApproval)); + continue; + } + let plan = serde_json::to_value(result.unwrap()).unwrap(); + assert_eq!(plan["mode"], serde_json::to_value(mode).unwrap()); + assert_eq!(plan["review_id"], review.review_id); + assert_eq!(plan["authority_receipt"], review.authority_receipt); + assert_eq!(plan["source_records_preserved"], true); + assert_eq!(plan["operations"][0]["item_key"], "A"); + assert_eq!(plan["operations"][1]["item_key"], "B"); + for operation in plan["operations"].as_array().unwrap() { + assert_eq!( + operation["rollback_collection_keys"], + operation["before_collection_keys"] + ); + assert_eq!(operation["rollback_tags"], operation["before_tags"]); + } + } + } +} diff --git a/crates/conceptweave-zotero/tests/golden_set_integrity_contract.rs b/crates/conceptweave-zotero/tests/golden_set_integrity_contract.rs index 7399394a..f14dce4e 100644 --- a/crates/conceptweave-zotero/tests/golden_set_integrity_contract.rs +++ b/crates/conceptweave-zotero/tests/golden_set_integrity_contract.rs @@ -541,6 +541,7 @@ fn source_metadata_mutations_invalidate_the_original_approval_before_verificatio "doi" => source.data.doi = "10.1/changed".into(), "tags" => source.data.tags.push(conceptweave_zotero::ItemTag { tag: "changed".into(), + tag_type: None, }), "collections" => source.data.collections.push("changed".into()), "type" => source.data.item_type = "attachment".into(), diff --git a/docs/CONTEXT_MAP.md b/docs/CONTEXT_MAP.md index e7c9cce1..073da7a1 100644 --- a/docs/CONTEXT_MAP.md +++ b/docs/CONTEXT_MAP.md @@ -6,10 +6,15 @@ - Semantic Discovery -> Model Validation: **Conformist to published candidate contract**; validation must not rewrite discovery evidence. - Model Validation -> Governance & Publication: **Customer/Supplier**; governance consumes deterministic validation receipts. - Governance & Publication -> Interoperability: **Published Language**; adapters consume immutable release contracts. -- Research Intake -> Governance & Publication: **Anti-Corruption Layer**; Intake validates source inventory, audit, receipt bindings and all duplicate operations before Governance verifies the complete independently issued review, including candidate membership and retained-source identity. The opaque authority receipt does not authorize source mutation. +- Research Intake -> Governance & Publication: **Anti-Corruption Layer**; Governance verifies complete duplicate and classification-write review sets before Intake emits canonical-key operations or write plans. Duplicate admission includes source inventory, audit, candidate membership and retained-source bindings. Opaque authority receipts and local plans are not proof of source mutation. ## External relationships +Classification-write admission reuses the same complete inventory/audit validator +and binds proposals, projected unclassified metadata and pending keys before +Governance verifies the entire reviewed change set. Duplicate membership and +full-text decisions retain their separate contracts; no new owner is introduced. + - Zotero Local API -> research evidence intake: **Anti-Corruption Layer into Semantic Discovery**. Zotero remains the bibliographic system of record; ConceptWeave consumes a version-pinned, read-only Local API snapshot and emits proposal evidence only. Item metadata, attachments, collection/tag truth, and future write authority remain in Zotero. No Zotero record becomes semantic authority without ConceptWeave validation/review/publication. - contextual-orchestrator -> Semantic Discovery: **Anti-Corruption Layer**. Model/provider envelopes never enter the domain model directly. - LineageWeave -> Source Observation: **Anti-Corruption Layer**. Inferred/proposed lineage remains explicitly non-authoritative until ConceptWeave governance evaluates it. diff --git a/docs/PRD.md b/docs/PRD.md index d4e7f4e0..6fd2466a 100644 --- a/docs/PRD.md +++ b/docs/PRD.md @@ -40,6 +40,11 @@ Validate syntax, identifiers, relationship cardinality, mapping completeness, du ### FR-5 Governed review +Research changes must use the evidence actually reviewed. Changed supporting +metadata or incomplete source inventories must stop a change plan before approval +is consumed. Older reviews lacking this evidence binding require fresh independent +approval; displaying a plan does not mean that the library has been changed. + A candidate cannot become authoritative solely because an LLM or automated extractor produced it. The publication lifecycle is Draft -> Proposed -> Validated -> Reviewed -> Published, with explicit rejection and supersession paths. ### FR-6 Publication @@ -66,6 +71,8 @@ Read a complete Zotero Local API observation with one consistent library version For every connected duplicate component, accept externally verified steward decisions selecting one component-level canonical item. Produce a local-only manifest that binds decisions to the raw snapshot, complete item revisions, exact duplicate membership, current proposals and retained source metadata. Reject missing or inconsistent source inventory and invalid decisions before requesting approval. Changed retained evidence requires fresh independent approval, even when duplicate members are unchanged. Record every component source revision plus before, after, and rollback canonical-key mappings. Classification preserves every Zotero source record. +Reviewed collection and tag changes default to a local dry-run plan. Each operation binds the authority receipt, server/library/item revisions, raw-snapshot digest, and complete before/after/rollback metadata. Zotero 9 execute requests fail closed. No plan contains credentials or permits `NeedsStewardReview`, source-record deletion, or attachment deletion. + Evaluate classifier quality only against a steward-reviewed local golden set whose governance receipt is externally verified and binds both the complete source/classifier-input snapshot and every current proposal field, in addition to the item-key/item-version coordinates. Same-version changes to unmodeled provider metadata, absent/default fields, classifier inputs, predictions or supporting evidence must invalidate the corresponding binding. Evaluation recomputes proposal identity before contacting governance; a locally changed digest cannot renew an approval. Legacy unbound approvals require reissuance, never automatic backfill. Abstention is a prediction outcome, never an approved truth label. Evaluation emits the verified library revision, rule revision, opaque snapshot and proposal digests, and aggregate counts for exact matches, abstentions, and per-disposition true-positive/predicted/expected totals; it must not copy Zotero keys, reviewer identity, or bibliographic text into the result. Every successful classification report includes aggregate evidence for snapshot coverage, proposal coverage, provenance completeness, abstentions, duplicate candidates, disposition totals, and zero unreported failures. diff --git a/docs/TRD.md b/docs/TRD.md index 04d4db58..4272c8e4 100644 --- a/docs/TRD.md +++ b/docs/TRD.md @@ -59,6 +59,15 @@ Evaluation must separate extraction recall, semantic correctness, structural cor ## 11. Zotero research intake +Classification write planning also calls the shared report validator before +external authority. Its required `proposal_digest` binds the reviewed v2 proposal, +unclassified-metadata and pending-key payloads and is retained in the plan. Legacy +receipts missing the field fail deserialization; blank bindings fail admission. +All existing local item/metadata errors retain precedence. Independent governance +authenticates the entire set; recomputing the digest cannot renew approval. This +does not replace full-text or duplicate-membership review contracts (ADR 0007). +Mode remains caller-supplied planning input, not receipt-bound execution authority. + `ClassificationReport.unclassified_items` retains every input record excluded from bibliographic classification, using the existing `ZoteroItem` metadata projection. Bibliographic proposals and this inventory are disjoint and together account for the observed record count on reader-admitted input. The existing child index is consumed once from bibliographic roots; records never reached remain in sorted `pending_source_item_keys`, including standalone roots, their descendants, orphan trees and cycles. The traversal is iterative, uses no new dependency and costs O(n log n) time/O(n) auxiliary space. It does not validate arbitrary offline input or preserve note bodies, attachment-specific fields and unknown provider JSON. Evaluation now calls `validate_classification_report` before governance: counts, disjoint complete key/version partitions, types, direct children and recomputed pending keys must agree. Equal partition count and one successful removal of each unique snapshot coordinate prove completeness. This owner has no original parent/type snapshot coordinates, so consistency is not source authentication. Later restoration, review, duplicate and write consumers must adopt this guard and require both inventory fields without empty legacy defaults. Keep bibliographic progress distinct from whole-library completion. Full-text report-digest changes require fresh bound verification, not capture rewriting. See [source-scope evidence and integration map](doctoring/zotero_source_scope.md). @@ -84,4 +93,4 @@ Structural, source, proposal, and label checks precede the external verifier. Bl Provider deserialization captures each complete JSON object before projecting metadata. Snapshot hashing serializes the domain marker `conceptweave-zotero-snapshot-v2` followed by key-ordered pairs of that canonical source JSON and the actual typed classifier input. Unknown nested fields, array order, and omitted-versus-explicit default fields remain bound; changing a typed input after decoding also changes the digest. Synthetic offline typed items have no captured provider object and bind an explicit absent-source value alongside their typed input. Earlier reduced-content digests remain historical evidence and cannot establish this complete-content contract; regenerate the report and review artifacts and obtain fresh approval before any release or approved write. A successful classification report carries an `audit_summary` whose snapshot, bibliographic, proposed-disposition, provenance-complete, abstention, duplicate-candidate, failure, and per-disposition counts are derived from the same in-memory immutable snapshot. Zotero item version zero remains a valid observed coordinate for never-synced Zotero 9 records; provenance completeness rejects a missing item key rather than inventing a positive-only version invariant. Reader failures return an error instead of a partial report; therefore a returned report records `failure_count=0` rather than hiding partial failures. -The report is local JSON and contains proposals rather than governance decisions. CLI output is restricted to a new direct child of canonical `/tmp` or the operating system temporary directory; relative paths, nested paths, existing paths, and symlinks are rejected, and create-new file semantics prevent overwrite/path-swap writes. Zotero 9 writes are unsupported; no mutation path exists in this slice. A future Zotero 10+ writer requires a separate reviewed change with a Local API key, stable server identity, fresh item/library version preconditions, item-by-item before/after receipts, and rollback evidence. +The report is local JSON and contains proposals rather than governance decisions. CLI output is restricted to a new direct child of canonical `/tmp` or the operating system temporary directory; relative paths, nested paths, existing paths, and symlinks are rejected, and create-new file semantics prevent overwrite/path-swap writes. Reviewed collection/tag changes can produce a pure local plan whose default mode is dry-run. The plan requires exact report and item preconditions, complete before/after/rollback arrays, externally verified authority, and preserved Zotero tag types. Zotero 9 execute mode fails closed and no mutation transport exists in this slice. A future Zotero 10+ writer must keep its Local API key outside serializable structures, revalidate server/library/item state before writing, and return item-level success, partial-failure, and rollback receipts. diff --git a/docs/UBIQUITOUS_LANGUAGE.md b/docs/UBIQUITOUS_LANGUAGE.md index 281497eb..085e2cd4 100644 --- a/docs/UBIQUITOUS_LANGUAGE.md +++ b/docs/UBIQUITOUS_LANGUAGE.md @@ -16,6 +16,8 @@ | Dimension | Governed categorical or temporal axis used to group/filter analytical facts. | | Measure | Governed calculation with explicit expression, grain, units, null semantics, and evidence. | | Semantic Steward | Authorized reviewer responsible for accepting or rejecting semantic meaning. | +| Reviewed Classification Change | Authorized complete replacement of one paper's collection and tag state, bound to its observed revision. | +| Classification Write Plan | Local deterministic artifact retaining the verified proposal/source-scope binding, exact preconditions and before/after/rollback metadata; dry-run by default, never proof of execution. | | Reviewed Duplicate Merge Set | Independently verified decisions selecting one canonical item per connected duplicate component, bound to exact source revisions, candidate membership, proposals and retained source scope. | | Authority Receipt | Opaque proof checked by the Governance & Publication boundary; it contains no reviewer identity or credential. | | Canonical-Key Operation | Reversible local mapping from every source in a connected duplicate component to one retained key, with complete reviewed revisions and the exact rollback mapping. | diff --git a/docs/UML.md b/docs/UML.md index bc4a7cf1..323f7341 100644 --- a/docs/UML.md +++ b/docs/UML.md @@ -65,5 +65,10 @@ sequenceDiagram Steward->>Intake: verified canonical-item decisions Intake->>Report: before/after/rollback identity manifest Report-->>Steward: reversible local mapping; source records preserved - Intake-->>Zotero: no mutation + Steward->>Intake: reviewed collection/tag changes and receipt + Intake->>Intake: existing item/metadata gates; shared inventory/audit validation; v2 binding + Intake->>Steward: independently verify complete reviewed set + Steward-->>Intake: authority result, not a replacement digest + Intake->>Report: dry-run write plan with exact rollback state + Intake-->>Zotero: Zotero 9 execute rejected; no mutation transport ``` diff --git a/docs/adr/0007-reviewed-zotero-write-plan.md b/docs/adr/0007-reviewed-zotero-write-plan.md new file mode 100644 index 00000000..d55337bc --- /dev/null +++ b/docs/adr/0007-reviewed-zotero-write-plan.md @@ -0,0 +1,57 @@ +# ADR 0007: Separate reviewed Zotero write planning from execution + +- Status: Proposed +- Date: 2026-09-04 +- Supersedes: the future-write deferral in ADR 0006; its read-only Zotero 9 decision remains valid + +## Context + +The installed-version and runtime-availability statements in this original context +and its original alternatives describe 2026-09-04, not current host state. Later +local evidence records Zotero 10.0.1; that alone establishes no write approval. + +Issue #8 requires classification changes to default to dry-run, preserve complete collection and tag state, reject stale review input, and make rollback reconstructable. The installed Zotero 9.0.6 Local API cannot write. Zotero 10+ writes additionally require a runtime-granted key, the same server identity, and fresh library/item versions. A planner can establish the review and recovery contract now without inventing authority or adding an unsafe Zotero 9 mutation path. + +## Decision + +ConceptWeave builds a local-only `ClassificationWritePlan` from an externally verified complete review set. Dry-run is the default. The review must match the exact Zotero version, server identity, library version, classifier revision, raw-snapshot digest, complete item-key/item-version coordinates, and observed collection/tag state. The plan retains the reviewed Zotero version used for execute eligibility. It rejects unknown or duplicate items, detached item revisions, blank or duplicate metadata, unsupported tag types, no-op changes, and `NeedsStewardReview` as a write decision. Operations are deterministic and retain complete before, after, and rollback states. Manual tag markers `None` and `0` are canonicalized to `None`; automatic tag type `1` is preserved. + +Execute planning fails closed for Zotero versions below 10. The plan contains no API key and performs no network call. A later adapter must preflight all operations and return item-level partial-success and rollback receipts; it must not claim cross-item transactionality or delete source records and attachments. + +## Consequences + +### Source-scope amendment (2026-09-06, Proposed) + +In the context of reviewed collection/tag replacement, facing a report whose +supporting title or inventory can change without changing its copied raw digest, +we decided for shared report admission and a required v2 proposal binding, and +against trusting only copied snapshot coordinates or duplicating inventory +validation, to reject changed evidence before consuming approval, accepting that +old receipts need fresh review and independent reissuance. + +Regression `dcde49e` demonstrated two failures: changed titles and inconsistent +observed counts still returned plans. Repair `c348278` checks the shared inventory, +audit and pending-source invariants, then compares the proposal digest, before +calling authority. Existing item/metadata error precedence remains unchanged. +The digest covers proposals, projected unclassified metadata and pending keys; +it does not claim full-text capture or duplicate-candidate authority. Duplicate +membership remains bound by the separate duplicate review contract. + +Governance must authenticate the complete reviewed set, including the binding +and requested changes. Test extension `1fe3d7d` exercises omitted/blank bindings, +retained plan identity, and a recomputed binding rejected by the original receipt. +No issuer, API write, automatic legacy backfill, or approval bypass is added. +Mode is a caller-supplied planning argument, not a field authenticated by this +reviewed set; Execute planning therefore supplies no execution authority. +This amendment remains Proposed until protected integration; later consumers must +adopt the required field without deriving fresh authority from serialized plans. + +- Review and rollback semantics can be tested on Zotero 9 without changing the library. +- Exact before-state checks prevent silent loss of unrelated collections or automatic-tag metadata. +- AC5 advances, while AC6 remains incomplete until an authenticated Zotero 10+ adapter and approved live write/rollback are verified. + +## Alternatives considered + +- Writing through Zotero 9 was rejected because the provider does not support it. +- Storing only collection/tag deltas was rejected because Zotero array updates are complete replacements and cannot prove lossless rollback. +- Adding the HTTP writer now was rejected because no Zotero 10+ runtime or approved local key is available for end-to-end verification. diff --git a/docs/adr/README.md b/docs/adr/README.md index bdd9b507..0170fbae 100644 --- a/docs/adr/README.md +++ b/docs/adr/README.md @@ -4,3 +4,4 @@ - [ADR 0002 — Evidence, truth, and publication lifecycle](0002-truth-publication-lifecycle.md) - [ADR 0003 — Standards and LLM engineering boundary](0003-standards-llm-boundary.md) - [ADR 0006 — Zotero research intake](0006-zotero-research-intake.md) +- [ADR 0007 — Reviewed Zotero write planning](0007-reviewed-zotero-write-plan.md) diff --git a/docs/product-technical-gap-baseline.md b/docs/product-technical-gap-baseline.md index 0533d51b..c221816f 100644 --- a/docs/product-technical-gap-baseline.md +++ b/docs/product-technical-gap-baseline.md @@ -89,6 +89,40 @@ Remaining work: mandatory adoption by restoration, worksheet, duplicate and writ ## DDD fitness constraints +### PR #13 source-scope integration checkpoint (2026-09-06) + +Normal merge `5df57a7` preserves write-plan head `b41217b` and source-scope +parent `3d2c252`. Two inherited test inputs lacked the optional tag type added +by write planning; explicitly retaining `None` restores their original manual-tag +semantics. Rust 1.98.0 workspace verification passed 94 tests across 18 result +suites, including two documentation tests. The initial shell selected Rust 1.97.1; +the verified invocation is `cargo +1.98.0 test --workspace`. + +Native Zotero visual inspection again showed 3,719 items and attachment icons. +This is display evidence, not approved reclassification or write evidence. +Write-plan source-scope validation and proposal-bound approval remain open; +this local integration is not a hosted-check, protected-merge, or release claim. + +### PR #13 write-scope repair checkpoint (2026-09-06) + +The preceding integration-only gap is locally repaired by `c348278` with tests +`dcde49e` (two RED failures) and `1fe3d7d` (binding/independent-receipt coverage). +Shared report admission now precedes authority, and the required v2 proposal +binding is retained in the plan. Legacy item/metadata errors retain precedence. +Independent static review found no blocking issue; it is not GitHub approval. + +Rust 1.98.0 workspace: 97 passing tests, 18 result suites including two doctests; +strict all-target Clippy, warnings-denied rustdoc and unchanged coverage gate pass. +Coverage: 169/169 functions, 1533/1533 source-normalized regions and 292/292 +normalized branches. Raw LLVM remains 1831/1852 lines, 2739/2777 regions and +255/292 branches, not 100%. Logs use `/tmp/conceptweave-pr13-write-scope-` with +`final.log`, `clippy.log`, `rustdoc.log` and `coverage.log` suffixes. + +PRD/TRD/ADR 0007 and DDD views retain separate metadata, duplicate-membership, +full-text and execution authority. No real decisions, approvals or Zotero writes +occurred. Protected merge, release, descendant adoption and actual reclassification +remain open; no local evidence is transferred to a remote or later head. + - No generic `utils/helpers/services/common` domain buckets. - Adapters remain outside the core domain model; external DTOs cross Anti-Corruption Layers. - Source Observation facts are not source-system business truth, and relational constraints are not semantic authority by themselves.