diff --git a/CHANGELOG.md b/CHANGELOG.md index 5c625695..7796fdf6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ All notable changes to ConceptWeave are documented here. ### Fixed +- 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. - Zotero research intake rejects a read whose records claim revisions newer than the library being observed, without dropping papers or changing their recorded revisions. diff --git a/crates/conceptweave-zotero/src/lib.rs b/crates/conceptweave-zotero/src/lib.rs index b17c8608..67c81568 100644 --- a/crates/conceptweave-zotero/src/lib.rs +++ b/crates/conceptweave-zotero/src/lib.rs @@ -175,16 +175,122 @@ pub struct ClassifiedItem { } /// A duplicate candidate group; no item is merged or deleted. -#[derive(Debug, Serialize)] +#[derive(Debug, Clone, PartialEq, Eq, Deserialize, Serialize)] pub struct DuplicateCandidate { /// Identity kind used for the candidate group. - pub identity_kind: &'static str, + pub identity_kind: String, /// Normalized identity value. pub normalized_identity: String, /// Zotero item keys sharing the identity. pub item_keys: Vec, } +/// One steward decision selecting the canonical identity for a duplicate cluster. +#[derive(Debug, Clone, PartialEq, Eq, Deserialize, Serialize)] +pub struct DuplicateMergeDecision { + /// Candidate identity kind from the classification report. + pub identity_kind: String, + /// Candidate normalized identity from the classification report. + pub normalized_identity: String, + /// Existing Zotero item retained as the canonical reference. + pub retained_item_key: String, +} + +/// Snapshot-bound duplicate decisions verified by the caller's governance boundary. +#[derive(Debug, Clone, PartialEq, Eq, Deserialize, Serialize)] +pub struct ReviewedDuplicateMergeSet { + /// Opaque steward review receipt. + pub review_id: String, + /// Opaque governance authority receipt; contains no person identity. + pub authority_receipt: String, + /// Exact Zotero library revision reviewed by the steward. + pub library_version: u64, + /// Exact classifier rule revision reviewed by the steward. + pub rule_revision: String, + /// Exact raw-snapshot digest reviewed by the steward. + pub snapshot_digest: String, + /// Required v2 identity of proposals, retained metadata and pending sources. + pub proposal_digest: String, + /// Exact item-key/item-version coordinates reviewed by the steward. + pub snapshot_items: Vec, + /// Exact duplicate membership reviewed by the steward. + pub duplicate_candidates: Vec, + /// Exactly one decision for every duplicate candidate. + pub decisions: Vec, +} + +/// One reversible canonical-key mapping; Zotero source records remain unchanged. +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +pub struct DuplicateMergeOperation { + /// Candidate identity kind. + pub identity_kind: String, + /// Candidate normalized identity retained only in the local manifest. + pub normalized_identity: String, + /// Steward-selected canonical Zotero item key. + pub retained_item_key: String, + /// Exact source revisions participating in the decision. + pub source_items: Vec, + /// Mapping before the reviewed canonicalization; every item maps to itself. + pub before_canonical_keys: BTreeMap, + /// Reviewed mapping after canonicalization; every item maps to the retained key. + pub after_canonical_keys: BTreeMap, + /// Exact inverse plan restoring the pre-review mapping. + pub rollback_canonical_keys: BTreeMap, +} + +/// Aggregate of reviewed, reversible duplicate identity operations. +#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +pub struct DuplicateMergeReviewManifest { + /// Opaque steward review receipt. + pub review_id: String, + /// Opaque governance authority receipt. + pub authority_receipt: String, + /// Exact Zotero library revision reviewed by the steward. + pub library_version: u64, + /// Exact classifier rule revision reviewed by the steward. + pub rule_revision: String, + /// Exact raw-snapshot digest shared with the reviewed decisions. + pub snapshot_digest: String, + /// Verified identity of the proposals and complete retained source scope. + pub proposal_digest: String, + /// Deterministically ordered canonical-key operations. + pub operations: Vec, + /// Classification never deletes or mutates Zotero source records. + pub source_records_preserved: bool, +} + +/// A fail-closed duplicate review contract violation. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum DuplicateReviewError { + /// Required review metadata or a decision is missing. + InvalidReview, + /// The review belongs to another raw snapshot. + SnapshotMismatch, + /// A decision does not identify a report candidate. + UnknownCandidate, + /// More than one decision targets the same candidate. + DuplicateDecision, + /// The retained key is not a member of the candidate cluster. + InvalidRetainedItem, + /// The caller's governance boundary rejected the complete review set. + UnverifiedApproval, +} + +impl fmt::Display for DuplicateReviewError { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + formatter.write_str(match self { + Self::InvalidReview => "duplicate review metadata or decisions are invalid", + Self::SnapshotMismatch => "duplicate review does not match the report snapshot", + Self::UnknownCandidate => "duplicate review contains an unknown candidate", + Self::DuplicateDecision => "duplicate review repeats a candidate decision", + Self::InvalidRetainedItem => "retained item is absent from its duplicate candidate", + Self::UnverifiedApproval => "duplicate review approval is unverified", + }) + } +} + +impl std::error::Error for DuplicateReviewError {} + /// Complete local classification report for one immutable library version. #[derive(Debug, Serialize)] pub struct ClassificationReport { @@ -584,6 +690,159 @@ where }) } +/// Builds a local-only reversible canonical-key manifest from steward-reviewed decisions. +/// +/// Validates source scope, receipt bindings and all operations before contacting +/// governance. The verifier must authenticate the complete independently issued +/// review, including candidate membership and the v2 proposal digest. Legacy +/// receipts require reapproval; digest recomputation alone grants no authority. +pub fn build_duplicate_merge_review_manifest( + report: &ClassificationReport, + reviewed: &ReviewedDuplicateMergeSet, + verify_review: F, +) -> Result +where + F: FnOnce(&ReviewedDuplicateMergeSet) -> bool, +{ + if reviewed.review_id.trim().is_empty() + || reviewed.authority_receipt.trim().is_empty() + || reviewed.snapshot_digest.trim().is_empty() + || reviewed.proposal_digest.trim().is_empty() + || reviewed.rule_revision.trim().is_empty() + || reviewed.decisions.len() != report.duplicate_candidates.len() + { + return Err(DuplicateReviewError::InvalidReview); + } + if reviewed.snapshot_digest != report.snapshot_digest + || reviewed.proposal_digest != classification_proposal_digest(report) + || reviewed.library_version != report.library_version + || reviewed.rule_revision != report.rule_revision + || reviewed.snapshot_items != report.snapshot_items + || reviewed.duplicate_candidates != report.duplicate_candidates + { + return Err(DuplicateReviewError::SnapshotMismatch); + } + validate_classification_report(report).map_err(|_| DuplicateReviewError::InvalidReview)?; + let item_revisions = report + .snapshot_items + .iter() + .map(|item| (item.item_key.as_str(), item.item_version)) + .collect::>(); + let candidates = report + .duplicate_candidates + .iter() + .map(|candidate| { + ( + ( + candidate.identity_kind.as_str(), + candidate.normalized_identity.as_str(), + ), + candidate, + ) + }) + .collect::>(); + let mut seen_candidates = BTreeSet::new(); + let mut operations = Vec::with_capacity(reviewed.decisions.len()); + + for decision in &reviewed.decisions { + let candidate_key = ( + decision.identity_kind.as_str(), + decision.normalized_identity.as_str(), + ); + if !seen_candidates.insert(candidate_key) { + return Err(DuplicateReviewError::DuplicateDecision); + } + let candidate = candidates + .get(&candidate_key) + .ok_or(DuplicateReviewError::UnknownCandidate)?; + // ponytail: quadratic component expansion is enough for a steward-sized review; + // replace with union-find only if measured duplicate sets become large. + let mut component_keys = candidate.item_keys.iter().collect::>(); + loop { + let previous_len = component_keys.len(); + for related in &report.duplicate_candidates { + if related + .item_keys + .iter() + .any(|item_key| component_keys.contains(item_key)) + { + component_keys.extend(&related.item_keys); + } + } + if component_keys.len() == previous_len { + break; + } + } + if !component_keys.contains(&decision.retained_item_key) { + return Err(DuplicateReviewError::InvalidRetainedItem); + } + if reviewed.decisions.iter().any(|related_decision| { + candidates + .get(&( + related_decision.identity_kind.as_str(), + related_decision.normalized_identity.as_str(), + )) + .is_some_and(|related_candidate| { + related_candidate + .item_keys + .iter() + .any(|item_key| component_keys.contains(item_key)) + && related_decision.retained_item_key != decision.retained_item_key + }) + }) { + return Err(DuplicateReviewError::InvalidReview); + } + + let source_items = component_keys + .iter() + .map(|item_key| { + item_revisions + .get(item_key.as_str()) + .map(|item_version| SnapshotItemRevision { + item_key: (*item_key).clone(), + item_version: *item_version, + }) + .ok_or(DuplicateReviewError::InvalidReview) + }) + .collect::, _>>()?; + let before_canonical_keys = component_keys + .iter() + .map(|item_key| ((*item_key).clone(), (*item_key).clone())) + .collect::>(); + let after_canonical_keys = component_keys + .iter() + .map(|item_key| ((*item_key).clone(), decision.retained_item_key.clone())) + .collect::>(); + operations.push(DuplicateMergeOperation { + identity_kind: decision.identity_kind.clone(), + normalized_identity: decision.normalized_identity.clone(), + retained_item_key: decision.retained_item_key.clone(), + source_items, + rollback_canonical_keys: before_canonical_keys.clone(), + before_canonical_keys, + after_canonical_keys, + }); + } + + if !verify_review(reviewed) { + return Err(DuplicateReviewError::UnverifiedApproval); + } + operations.sort_by(|left, right| { + (&left.identity_kind, &left.normalized_identity) + .cmp(&(&right.identity_kind, &right.normalized_identity)) + }); + Ok(DuplicateMergeReviewManifest { + review_id: reviewed.review_id.clone(), + authority_receipt: reviewed.authority_receipt.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 { @@ -1211,7 +1470,7 @@ fn duplicate_candidates(items: &[&ZoteroItem]) -> Vec { .into_iter() .filter_map(|((identity_kind, normalized_identity), item_keys)| { (item_keys.len() > 1).then_some(DuplicateCandidate { - identity_kind, + identity_kind: identity_kind.into(), normalized_identity, item_keys, }) diff --git a/crates/conceptweave-zotero/tests/duplicate_merge_review_components.rs b/crates/conceptweave-zotero/tests/duplicate_merge_review_components.rs new file mode 100644 index 00000000..21f95876 --- /dev/null +++ b/crates/conceptweave-zotero/tests/duplicate_merge_review_components.rs @@ -0,0 +1,188 @@ +use conceptweave_zotero::{ + DuplicateMergeDecision, DuplicateReviewError, ItemData, ReviewedDuplicateMergeSet, ZoteroItem, + build_duplicate_merge_review_manifest, classify_snapshot, +}; + +fn item(key: &str, version: u64, title: &str, doi: &str) -> ZoteroItem { + ZoteroItem { + source_record: None, + key: key.into(), + version, + data: ItemData { + item_type: "journalArticle".into(), + title: title.into(), + abstract_note: String::new(), + doi: doi.into(), + parent_item: String::new(), + collections: vec![], + tags: vec![], + }, + } +} + +#[test] +fn transitive_duplicate_component_accepts_one_component_level_canonical_key() { + let report = classify_snapshot( + "9.0.6".into(), + None, + 42, + vec![ + item("A", 1, "Alpha", "10.1000/ab"), + item("B", 2, "Bridge", "10.1000/ab"), + item("C", 3, "Bridge", "10.1000/cd"), + item("D", 4, "Delta", "10.1000/cd"), + ], + ); + assert_eq!(report.duplicate_candidates.len(), 3); + + let reviewed = ReviewedDuplicateMergeSet { + review_id: "review-transitive-component".into(), + authority_receipt: "authority-transitive-component".into(), + library_version: report.library_version, + rule_revision: report.rule_revision.into(), + snapshot_digest: report.snapshot_digest.clone(), + proposal_digest: conceptweave_zotero::classification_proposal_digest(&report), + snapshot_items: report.snapshot_items.clone(), + duplicate_candidates: report.duplicate_candidates.clone(), + decisions: report + .duplicate_candidates + .iter() + .map(|candidate| DuplicateMergeDecision { + identity_kind: candidate.identity_kind.clone(), + normalized_identity: candidate.normalized_identity.clone(), + retained_item_key: "A".into(), + }) + .collect(), + }; + + let manifest = build_duplicate_merge_review_manifest(&report, &reviewed, |_| true).expect( + "one steward-selected canonical key must be valid across a transitive duplicate component", + ); + + assert_eq!(manifest.operations.len(), 3); + for operation in manifest.operations { + assert_eq!(operation.retained_item_key, "A"); + assert_eq!(operation.source_items.len(), 4); + assert!( + operation + .source_items + .iter() + .any(|item| item.item_key == "A") + ); + assert!( + operation + .after_canonical_keys + .values() + .all(|canonical_key| canonical_key == "A") + ); + } +} + +#[test] +fn duplicate_review_rejects_blank_snapshot_item_identity() { + let mut report = classify_snapshot( + "9.0.6".into(), + None, + 42, + vec![ + item("", 1, "Blank identity", "10.1000/blank-key"), + item("B", 2, "Other identity", "10.1000/blank-key"), + ], + ); + assert_eq!(report.duplicate_candidates.len(), 1); + + let candidate = &report.duplicate_candidates[0]; + let reviewed = ReviewedDuplicateMergeSet { + review_id: "review-blank-key".into(), + authority_receipt: "authority-blank-key".into(), + library_version: report.library_version, + rule_revision: report.rule_revision.into(), + snapshot_digest: report.snapshot_digest.clone(), + proposal_digest: conceptweave_zotero::classification_proposal_digest(&report), + snapshot_items: report.snapshot_items.clone(), + duplicate_candidates: report.duplicate_candidates.clone(), + decisions: vec![DuplicateMergeDecision { + identity_kind: candidate.identity_kind.clone(), + normalized_identity: candidate.normalized_identity.clone(), + retained_item_key: "B".into(), + }], + }; + + assert_eq!( + build_duplicate_merge_review_manifest(&report, &reviewed, |_| true), + Err(DuplicateReviewError::InvalidReview), + "blank Zotero keys are not stable provenance identities and must fail closed before manifest materialization" + ); + + report.snapshot_items.remove(0); + assert_eq!( + build_duplicate_merge_review_manifest(&report, &reviewed, |_| true), + Err(DuplicateReviewError::SnapshotMismatch), + "every duplicate candidate must remain bound to its reviewed snapshot revision" + ); +} + +#[test] +fn duplicate_review_binds_every_item_revision_to_the_reviewed_snapshot() { + let mut report = classify_snapshot( + "9.0.6".into(), + None, + 42, + vec![ + item("A", 1, "Same title", "10.1000/same"), + item("B", 2, "Same title", "10.1000/same"), + item("C", 3, "Unrelated title", "10.1000/unrelated"), + ], + ); + let mut reviewed = ReviewedDuplicateMergeSet { + review_id: "review-revisions".into(), + authority_receipt: "authority-revisions".into(), + library_version: report.library_version, + rule_revision: report.rule_revision.into(), + snapshot_digest: report.snapshot_digest.clone(), + proposal_digest: conceptweave_zotero::classification_proposal_digest(&report), + snapshot_items: report.snapshot_items.clone(), + duplicate_candidates: report.duplicate_candidates.clone(), + decisions: report + .duplicate_candidates + .iter() + .map(|candidate| DuplicateMergeDecision { + identity_kind: candidate.identity_kind.clone(), + normalized_identity: candidate.normalized_identity.clone(), + retained_item_key: "A".into(), + }) + .collect(), + }; + report.snapshot_items[0].item_version += 1; + + assert_eq!( + build_duplicate_merge_review_manifest(&report, &reviewed, |_| true), + Err(DuplicateReviewError::SnapshotMismatch) + ); + + report.snapshot_items[0].item_version -= 1; + for decision in &mut reviewed.decisions { + decision.retained_item_key = "B".into(); + } + for candidate in &mut report.duplicate_candidates { + candidate.item_keys[0] = "C".into(); + } + assert_eq!( + build_duplicate_merge_review_manifest(&report, &reviewed, |_| true), + Err(DuplicateReviewError::SnapshotMismatch) + ); + + reviewed.duplicate_candidates = report.duplicate_candidates.clone(); + for (candidate, reviewed_candidate) in report + .duplicate_candidates + .iter_mut() + .zip(&mut reviewed.duplicate_candidates) + { + candidate.item_keys[0] = "missing".into(); + reviewed_candidate.item_keys[0] = "missing".into(); + } + assert_eq!( + build_duplicate_merge_review_manifest(&report, &reviewed, |_| true), + Err(DuplicateReviewError::InvalidReview) + ); +} diff --git a/crates/conceptweave-zotero/tests/duplicate_merge_review_manifest.rs b/crates/conceptweave-zotero/tests/duplicate_merge_review_manifest.rs new file mode 100644 index 00000000..ed085596 --- /dev/null +++ b/crates/conceptweave-zotero/tests/duplicate_merge_review_manifest.rs @@ -0,0 +1,377 @@ +use conceptweave_zotero::{ + Disposition, DuplicateMergeDecision, DuplicateReviewError, ItemData, ReviewedDuplicateMergeSet, + ZoteroItem, build_duplicate_merge_review_manifest, classify_snapshot, +}; + +fn item(key: &str, version: u64, title: &str, doi: &str) -> ZoteroItem { + ZoteroItem { + source_record: None, + key: key.into(), + version, + data: ItemData { + item_type: "journalArticle".into(), + title: title.into(), + abstract_note: String::new(), + doi: doi.into(), + parent_item: String::new(), + collections: vec![], + tags: vec![], + }, + } +} + +fn report() -> conceptweave_zotero::ClassificationReport { + classify_snapshot( + "9.0.6".into(), + None, + 42, + vec![ + item("A", 7, "Ontology Learning", "10.1000/example"), + item( + "B", + 9, + "Ontology Learning Copy", + "https://doi.org/10.1000/example", + ), + item("C", 3, "Shared Ontology Title", ""), + item("D", 4, "Shared Ontology Title", ""), + { + let mut note = item("NOTE", 0, "Standalone source", ""); + note.data.item_type = "note".into(); + note + }, + ], + ) +} + +fn reviewed(report: &conceptweave_zotero::ClassificationReport) -> ReviewedDuplicateMergeSet { + ReviewedDuplicateMergeSet { + review_id: "review-duplicate-1".into(), + authority_receipt: "authority-receipt-1".into(), + library_version: report.library_version, + rule_revision: report.rule_revision.into(), + snapshot_digest: report.snapshot_digest.clone(), + proposal_digest: conceptweave_zotero::classification_proposal_digest(report), + snapshot_items: report.snapshot_items.clone(), + duplicate_candidates: report.duplicate_candidates.clone(), + decisions: vec![ + DuplicateMergeDecision { + identity_kind: "title".into(), + normalized_identity: "shared ontology title".into(), + retained_item_key: "C".into(), + }, + DuplicateMergeDecision { + identity_kind: "doi".into(), + normalized_identity: "10.1000/example".into(), + retained_item_key: "A".into(), + }, + ], + } +} + +#[test] +fn reviewed_duplicate_decision_has_exact_before_after_and_rollback_mappings() { + let report = report(); + let manifest = build_duplicate_merge_review_manifest(&report, &reviewed(&report), |_| true) + .expect("synthetic reviewed duplicate must produce a manifest"); + + assert!(manifest.source_records_preserved); + assert_eq!(manifest.authority_receipt, "authority-receipt-1"); + assert_eq!(manifest.library_version, report.library_version); + assert_eq!(manifest.rule_revision, report.rule_revision); + assert_eq!( + manifest.proposal_digest, + conceptweave_zotero::classification_proposal_digest(&report) + ); + assert_eq!(manifest.operations.len(), 2); + assert_eq!(manifest.operations[0].identity_kind, "doi"); + assert_eq!(manifest.operations[1].identity_kind, "title"); + let operation = manifest + .operations + .iter() + .find(|operation| operation.identity_kind == "doi") + .expect("DOI duplicate operation must exist"); + assert_eq!(operation.retained_item_key, "A"); + assert_eq!(operation.source_items[0].item_version, 7); + assert_eq!(operation.source_items[1].item_version, 9); + assert_eq!(operation.before_canonical_keys["B"], "B"); + assert_eq!(operation.after_canonical_keys["B"], "A"); + assert_eq!( + operation.rollback_canonical_keys, + operation.before_canonical_keys + ); + assert_eq!( + report.classified_items[0].proposed_disposition, + Disposition::Generation + ); + let serialized = serde_json::to_value(&manifest).expect("manifest must serialize"); + assert_eq!( + serialized["operations"][0]["rollback_canonical_keys"], + serialized["operations"][0]["before_canonical_keys"] + ); +} + +#[test] +fn duplicate_scope_and_decisions_fail_before_governance() { + for mutation in 0..7 { + let mut report = report(); + let mut review = reviewed(&report); + match mutation { + 0 => report.observed_item_count += 1, + 1 => report.unclassified_items.clear(), + 2 => report.pending_source_item_keys.clear(), + 3 => report.audit_summary.failure_count = 1, + 4 => review.decisions[0].retained_item_key = "missing".into(), + 5 => review.decisions[0].normalized_identity = "missing".into(), + _ => review.decisions[0] = review.decisions[1].clone(), + } + let called = std::cell::Cell::new(false); + assert!( + build_duplicate_merge_review_manifest(&report, &review, |_| { + called.set(true); + true + }) + .is_err(), + "invalid scope or decision {mutation} accepted" + ); + assert!( + !called.get(), + "invalid scope or decision {mutation} reached governance" + ); + } +} + +#[test] +fn retained_source_change_invalidates_duplicate_approval() { + let mut report = report(); + let review = reviewed(&report); + report.unclassified_items[0].data.title = "Changed standalone evidence".into(); + assert_eq!( + build_duplicate_merge_review_manifest(&report, &review, |_| panic!( + "changed scope reached governance" + )), + Err(DuplicateReviewError::SnapshotMismatch) + ); +} + +#[test] +fn duplicate_receipt_requires_explicit_source_scope_binding() { + let mut legacy = serde_json::to_value(reviewed(&report())).unwrap(); + legacy.as_object_mut().unwrap().remove("proposal_digest"); + assert!(serde_json::from_value::(legacy).is_err()); +} + +#[test] +fn rewritten_duplicate_scope_receipt_cannot_reuse_independent_approval() { + let mut report = report(); + let mut review = reviewed(&report); + let original_review = review.clone(); + report.unclassified_items[0].data.title = "Changed retained evidence".into(); + review.proposal_digest = conceptweave_zotero::classification_proposal_digest(&report); + let called = std::cell::Cell::new(0); + assert_eq!( + build_duplicate_merge_review_manifest(&report, &review, |candidate| { + called.set(called.get() + 1); + candidate == &original_review + }), + Err(DuplicateReviewError::UnverifiedApproval) + ); + assert_eq!(called.get(), 1); + + review.proposal_digest.clear(); + assert_eq!( + build_duplicate_merge_review_manifest(&report, &review, |_| panic!( + "blank binding reached governance" + )), + Err(DuplicateReviewError::InvalidReview) + ); +} + +#[test] +fn duplicate_review_contract_fails_closed() { + let report = report(); + let mut review = reviewed(&report); + + assert_eq!( + build_duplicate_merge_review_manifest(&report, &review, |_| false), + Err(DuplicateReviewError::UnverifiedApproval) + ); + review.snapshot_digest = "sha256:stale".into(); + assert_eq!( + build_duplicate_merge_review_manifest(&report, &review, |_| true), + Err(DuplicateReviewError::SnapshotMismatch) + ); + review = reviewed(&report); + review.decisions[1].retained_item_key = "missing".into(); + assert_eq!( + build_duplicate_merge_review_manifest(&report, &review, |_| true), + Err(DuplicateReviewError::InvalidRetainedItem) + ); + review = reviewed(&report); + review.decisions[1].normalized_identity = "missing".into(); + assert_eq!( + build_duplicate_merge_review_manifest(&report, &review, |_| true), + Err(DuplicateReviewError::UnknownCandidate) + ); + review = reviewed(&report); + review.decisions[1] = review.decisions[0].clone(); + assert_eq!( + build_duplicate_merge_review_manifest(&report, &review, |_| true), + Err(DuplicateReviewError::DuplicateDecision) + ); + review.decisions.clear(); + assert_eq!( + build_duplicate_merge_review_manifest(&report, &review, |_| true), + Err(DuplicateReviewError::InvalidReview) + ); + review = reviewed(&report); + review.authority_receipt.clear(); + assert_eq!( + build_duplicate_merge_review_manifest(&report, &review, |_| true), + Err(DuplicateReviewError::InvalidReview) + ); + for clear_field in [ + |review: &mut ReviewedDuplicateMergeSet| review.review_id.clear(), + |review: &mut ReviewedDuplicateMergeSet| review.snapshot_digest.clear(), + |review: &mut ReviewedDuplicateMergeSet| review.rule_revision.clear(), + ] { + review = reviewed(&report); + clear_field(&mut review); + assert_eq!( + build_duplicate_merge_review_manifest(&report, &review, |_| true), + Err(DuplicateReviewError::InvalidReview) + ); + } + review = reviewed(&report); + review.library_version += 1; + assert_eq!( + build_duplicate_merge_review_manifest(&report, &review, |_| true), + Err(DuplicateReviewError::SnapshotMismatch) + ); + review = reviewed(&report); + review.rule_revision = "stale-rule".into(); + assert_eq!( + build_duplicate_merge_review_manifest(&report, &review, |_| true), + Err(DuplicateReviewError::SnapshotMismatch) + ); + + for (error, fragment) in [ + (DuplicateReviewError::InvalidReview, "invalid"), + (DuplicateReviewError::SnapshotMismatch, "snapshot"), + (DuplicateReviewError::UnknownCandidate, "unknown"), + (DuplicateReviewError::DuplicateDecision, "repeats"), + (DuplicateReviewError::InvalidRetainedItem, "absent"), + (DuplicateReviewError::UnverifiedApproval, "unverified"), + ] { + assert!(error.to_string().contains(fragment)); + } +} + +#[test] +fn overlapping_duplicate_groups_require_one_consistent_canonical_choice() { + let report = classify_snapshot( + "9.0.6".into(), + None, + 42, + vec![ + item("A", 7, "Same Identity", "10.1000/overlap"), + item("B", 9, "Same Identity", "10.1000/overlap"), + ], + ); + assert_eq!(report.duplicate_candidates.len(), 2); + + let reviewed = ReviewedDuplicateMergeSet { + review_id: "review-overlap".into(), + authority_receipt: "authority-overlap".into(), + library_version: report.library_version, + rule_revision: report.rule_revision.into(), + snapshot_digest: report.snapshot_digest.clone(), + proposal_digest: conceptweave_zotero::classification_proposal_digest(&report), + snapshot_items: report.snapshot_items.clone(), + duplicate_candidates: report.duplicate_candidates.clone(), + decisions: vec![ + DuplicateMergeDecision { + identity_kind: "doi".into(), + normalized_identity: "10.1000/overlap".into(), + retained_item_key: "A".into(), + }, + DuplicateMergeDecision { + identity_kind: "title".into(), + normalized_identity: "same identity".into(), + retained_item_key: "B".into(), + }, + ], + }; + + assert_eq!( + build_duplicate_merge_review_manifest(&report, &reviewed, |_| true), + Err(DuplicateReviewError::InvalidReview), + "overlapping duplicate groups cannot emit conflicting A->B and B->A canonical mappings" + ); +} + +#[test] +fn duplicate_review_rejects_ambiguous_snapshot_key_revisions() { + let report = classify_snapshot( + "9.0.6".into(), + None, + 42, + vec![ + item("DUPKEY", 7, "Same Identity", "10.1000/duplicate-key"), + item("DUPKEY", 9, "Same Identity", "10.1000/duplicate-key"), + ], + ); + assert_eq!(report.snapshot_items.len(), 2); + assert!(!report.duplicate_candidates.is_empty()); + + let reviewed = ReviewedDuplicateMergeSet { + review_id: "review-duplicate-key".into(), + authority_receipt: "authority-duplicate-key".into(), + library_version: report.library_version, + rule_revision: report.rule_revision.into(), + snapshot_digest: report.snapshot_digest.clone(), + proposal_digest: conceptweave_zotero::classification_proposal_digest(&report), + snapshot_items: report.snapshot_items.clone(), + duplicate_candidates: report.duplicate_candidates.clone(), + decisions: report + .duplicate_candidates + .iter() + .map(|candidate| DuplicateMergeDecision { + identity_kind: candidate.identity_kind.clone(), + normalized_identity: candidate.normalized_identity.clone(), + retained_item_key: "DUPKEY".into(), + }) + .collect(), + }; + + assert_eq!( + build_duplicate_merge_review_manifest(&report, &reviewed, |_| true), + Err(DuplicateReviewError::InvalidReview), + "duplicate Zotero keys must fail before a BTreeMap can collapse distinct observed revisions" + ); +} + +#[test] +fn duplicate_review_domain_language_and_governance_handoff_are_documented() { + let ubiquitous_language = include_str!("../../../docs/UBIQUITOUS_LANGUAGE.md"); + for term in [ + "Reviewed Duplicate Merge Set", + "Authority Receipt", + "Canonical-Key Operation", + ] { + assert!( + ubiquitous_language.contains(term), + "UBIQUITOUS_LANGUAGE.md must define `{term}`" + ); + } + + let context_map = include_str!("../../../docs/CONTEXT_MAP.md"); + assert!( + context_map.contains("Research Intake -> Governance & Publication"), + "CONTEXT_MAP.md must name the Research Intake to Governance & Publication verification handoff" + ); + assert!( + context_map.contains("Anti-Corruption Layer"), + "the duplicate-review governance handoff must preserve an explicit ACL boundary" + ); +} diff --git a/docs/CONTEXT_MAP.md b/docs/CONTEXT_MAP.md index bc769582..e7c9cce1 100644 --- a/docs/CONTEXT_MAP.md +++ b/docs/CONTEXT_MAP.md @@ -6,6 +6,7 @@ - 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. ## External relationships diff --git a/docs/PRD.md b/docs/PRD.md index 791c4cee..d4e7f4e0 100644 --- a/docs/PRD.md +++ b/docs/PRD.md @@ -64,8 +64,9 @@ Library reads must finish within a bounded observation window or fail visibly wi Read a complete Zotero Local API observation with one consistent library version and propose exactly one research disposition for every top-level bibliographic item. This consistency check does not establish an atomic provider snapshot. A record claiming a revision newer than the observed library invalidates the complete read; it must not be omitted or assigned a different revision to make the read pass. Each proposal retains the item key/version, exact matched metadata values, rule revision, linked child records, and any model receipt. Weak evidence and evidence that matches multiple specific disposition families must abstain into steward review. Duplicate DOI/title identities are review candidates only: intake never merges, deletes, or silently mutates Zotero records. -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. +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. +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. ## 6. First vertical slice diff --git a/docs/TRD.md b/docs/TRD.md index aa2a3585..04d4db58 100644 --- a/docs/TRD.md +++ b/docs/TRD.md @@ -73,6 +73,8 @@ After count/byte validation and before accumulating each metadata page, every re Every top-level bibliographic record receives exactly one proposed disposition. `NeedsStewardReview` also records a deterministic abstention reason so missing classification metadata, vocabulary unsupported by the current deterministic rules, and present-but-unmatched metadata are distinguishable. DOI duplicate identity normalization treats bare DOI values, `doi:`, `doi.org`, and legacy `dx.doi.org` resolver forms as the same identity when their normalized DOI is equal. +Duplicate decisions are independent of subject labels but share the complete source-evidence admission boundary. A reviewed set must match the raw-snapshot digest, item revisions, exact candidate membership and required v2 `proposal_digest`. Receipt comparison retains `SnapshotMismatch` precedence; shared structural/audit admission and all component/decision checks then run before the external verifier. The verifier authenticates the entire independently issued set, not a locally recomputed digest. Missing scope bindings fail deserialization and blank bindings fail admission; no legacy default or automatic reapproval exists. The manifest retains the verified proposal digest. Every operation records all component item revisions, before/after identity maps and exact rollback. Zotero records remain unchanged. + Golden-set evaluation accepts only a governance receipt verified with the complete reviewed set by a caller-owned authorization boundary. Its library version, rule revision, canonical SHA-256 content digest, and every observed parent/child item-key/item-version identity must bind the classification report. The snapshot digest covers every raw Zotero item in canonical key order. A separate required `proposal_digest` binds every field of every proposed item, including predictions, supporting evidence, and proposals outside a reviewed sample. `classification_proposal_digest` computes SHA-256 over compact JSON containing the `conceptweave-classification-proposals-v1` domain marker and proposal records sorted by item key and revision. It uses the current records, not a report's self-declared digest or a second stored source snapshot. Governance must issue and independently verify both digests together with the labels; a locally recomputed replacement digest cannot renew an old approval. Legacy approvals missing this field fail closed and require reissuance, not automatic backfill. The proposal-only v1 format above describes the previous receipt contract. The current source-scope amendment supersedes it with `conceptweave-classification-proposals-v2`: compact JSON binds the marker, sorted proposals, key/version-sorted projected unclassified records, and sorted pending keys. Old v1 receipts fail before governance; locally rewritten digests cannot renew approval. This is not a lossless full-text digest. @@ -80,7 +82,6 @@ The proposal-only v1 format above describes the previous receipt contract. The c Structural, source, proposal, and label checks precede the external verifier. Blank, duplicate, unknown, stale, content-mismatched, prediction-mismatched, label-mismatched, or abstention-as-truth inputs fail closed. The aggregate result retains the verified library version, rule revision, and opaque snapshot/proposal digests, but contains no item keys, reviewer identity, or bibliographic text. Production authorization remains Keyverse/governance-owned; this crate passes the complete reviewed labels and approval bindings to that boundary instead of minting authority. 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. diff --git a/docs/UBIQUITOUS_LANGUAGE.md b/docs/UBIQUITOUS_LANGUAGE.md index 0c1d0c25..281497eb 100644 --- a/docs/UBIQUITOUS_LANGUAGE.md +++ b/docs/UBIQUITOUS_LANGUAGE.md @@ -16,3 +16,6 @@ | 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 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 0c1a99b4..bc4a7cf1 100644 --- a/docs/UML.md +++ b/docs/UML.md @@ -62,5 +62,8 @@ sequenceDiagram Intake->>Intake: validate complete partitions and recompute pending ancestry Intake->>Intake: verify v2 proposal and retained-source binding Note over Intake,Steward: Only locally valid reports reach independent governance verification + 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 ``` diff --git a/docs/adr/0006-zotero-research-intake.md b/docs/adr/0006-zotero-research-intake.md index 69299e6e..22584a05 100644 --- a/docs/adr/0006-zotero-research-intake.md +++ b/docs/adr/0006-zotero-research-intake.md @@ -19,6 +19,8 @@ The adapter links child records, emits exactly one deterministic proposed dispos Matched metadata values are copied into the local-only evidence receipt for replay. This is necessary for abstract-only matches because a later Zotero revision cannot reconstruct the exact text used for an earlier proposal from item key/version alone. DOI/title matches remain reversible duplicate candidates, including legacy `dx.doi.org` resolver forms. +Duplicate candidates become canonical references only through externally verified steward decisions bound to the raw digest, complete item-key/item-version snapshot, and exact candidate membership. Overlapping candidates form one connected component and must select one component-level canonical item. Every resulting operation retains all component source revisions and complete before/after/rollback key mappings. It changes downstream identity resolution only; classification does not merge, delete, or mutate Zotero source records. + Classifier quality is measured only against local steward-reviewed labels whose complete reviewed set is verified outside this crate and bound to the exact library version, rule revision, canonical SHA-256 raw-snapshot digest, and every observed parent/child item-key/item-version coordinate. `NeedsStewardReview` is an abstention prediction and cannot be approved truth. Evaluation returns the verified revisions and opaque digest with aggregate integer evidence; Zotero keys, reviewer identity, and bibliographic text are omitted. Missing, stale, content- or label-mismatched, unverified, unknown, duplicate, or invalid review identities fail closed. Every successful report includes an aggregate audit summary computed from the same captured snapshot. Zotero 9 item version zero is preserved as a valid never-synced source coordinate, not treated as missing provenance. Partial reads never produce a report, so successful output explicitly records zero failures alongside snapshot, proposal, provenance, abstention, duplicate, and per-disposition totals. @@ -73,6 +75,16 @@ GREEN source `f6735b585022aac1c8ceff86c150d9b64fd77ec2` passed 68 tests across 1 ## Alternatives considered +### September 6 duplicate source-scope admission (Proposed) + +PR #12 already binds exact candidate membership and complete item revisions in `ReviewedDuplicateMergeSet`; the prior audit-owner concern about unbound duplicate authority therefore does not describe this consumer. Its real remaining gap was that retained metadata and inventory were absent from its receipt, and external verification preceded local decision checks. RED `4656d6b` reproduced missing legacy scope binding, malformed inventory accepted, and altered standalone evidence reaching governance. + +We reused the existing shared report validator and v2 proposal digest. Duplicate receipts and resulting manifests require `proposal_digest`; original candidate membership binding remains separate and unchanged. Missing bindings fail deserialization, blank bindings fail admission, and locally rewritten bindings still require independently issued approval of the complete set. All component and decision checks precede the external verifier. No source mutation, new authority issuer, digest algorithm or dependency was added. Snapshot mismatch takes precedence over structural errors to preserve existing caller contracts; `fc0465e` restores this after the first implementation exposed two exact-error regressions. + +Rejected alternatives were another source validator, copying the source snapshot into each operation, mixing derived counters into source identity, or defaulting legacy receipts to a recomputed digest. Each duplicates responsibility or weakens review evidence. The cost is explicit reapproval of older duplicate receipts and rejection after changed retained evidence, even when candidate membership stays constant. `0c825d9` proves that rewriting the digest does not reuse approval and the manifest retains the verified binding. Subsequent restored-report/worksheet/write owners must adopt the required fields without granting authority by downcasting full-text evidence. + +The isolated baseline at `a4a7c2d` passed 68 tests/17 suites; normal integration `b758991` preserved it and parent `6dff8c2`, passing 83/17. Current runtime `fc0465e` plus tests `0c825d9` passed 87/17 including two doctests. Independent duplicate tests passed 12/12 with no actionable finding. Unchanged pinned coverage passed 153/153 functions, 1,294/1,294 normalized owned regions and 220/220 normalized branches; raw 1,649/1,658 lines, 2,516/2,536 regions and 200/220 branches remain below 100%. Rustdoc/release passed; strict Clippy found one test-only redundant borrow, corrected in `5fff9d0` without suppression. Hosted checks, protected merge, release and genuine reviewed reclassification remain unproven. + ### September 6 derived audit repair (Proposed) Live PR #11 findings [3934799129](https://github.com/ContextualWisdomLab/ConceptWeave/pull/11#discussion_r3934799129) and [3934994550](https://github.com/ContextualWisdomLab/ConceptWeave/pull/11#discussion_r3934994550) were rechecked after source-scope integration. Source identity must not absorb derived audit fields, but accepting arbitrary audit values alongside a verified report can still misstate completeness. RED `ebdd852` proved both forged audit reaching governance and duplicate source keys counted as complete provenance. diff --git a/docs/product-technical-gap-baseline.md b/docs/product-technical-gap-baseline.md index 49eb6b43..0533d51b 100644 --- a/docs/product-technical-gap-baseline.md +++ b/docs/product-technical-gap-baseline.md @@ -56,6 +56,8 @@ Protected central source is `.github/main@b5efbc2762e472e4a380b0503b1f050f76fbb0 ## September 6 source-scope admission checkpoint +PR #12 successor runtime `fc0465e` now uses the shared inventory/audit validator and required v2 scope binding before independent duplicate governance. The existing exact candidate-membership receipt already covers duplicate selection; it is not an unprotected authority gap in this consumer. RED `4656d6b` reproduced three retained-source admission bypasses, now rejected. Current local result: 87 tests/17 suites, independent duplicate review 12/12, pinned coverage 153/153 functions, 1,294/1,294 normalized regions and 220/220 normalized branches. Raw coverage remains 1,649/1,658 lines, 2,516/2,536 regions, 200/220 branches. See the Proposed ADR 0006 amendment for history, compatibility and remaining gates. Later restored-report/worksheet/write integration is still required; current root PR #39 has not adopted these changes. + PR #11 local successor checkpoint `23178a9768aa216692d77918d56357ae1269535c` normally inherits #10 `fdf8b8d70c05bcb76c55cb6336c9bf31b5e42ce4` while preserving prior #11 `1dc032598b41a35d52c09d8690c871e07365d7e3`. The stable isolated baseline passed 60 tests/15 suites; an earlier run contaminated by merge timing is invalid evidence. RED `ebdd852` reproduced forged derived audit and ambiguous provenance totals. Extracted audit computation now counts unique parent/child identities and is recomputed before governance. Final local tests passed 75/15 suites including two doctests; independent integrity review passed 17 tests with no new regression. Strict Clippy passed at unchanged runtime `935e035`. Pinned coverage is still running; no inherited coverage claim or remote PR update is made here. The pinned coverage run subsequently finished with exit 0: 140/140 functions, 1,101/1,101 normalized owned regions, 182/182 normalized branches. Raw lines 1,502/1,505, regions 2,332/2,343 and branches 177/182 remain below 100%. The coverage script and exclusions are unchanged; the earlier pending sentence records chronology, not current execution state.