diff --git a/CHANGELOG.md b/CHANGELOG.md index 4454ea38..c3994ce5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,7 @@ All notable changes to ConceptWeave are documented here. - A snapshot-bound steward worksheet with one blank decision per bibliographic item and no duplicated bibliographic text. - An explicit `--worksheet` CLI mode that writes the live worksheet with owner-only report protections. - Fail-closed conversion from a fully decided worksheet to the existing externally verified golden-set boundary. +- Lossless owner-only classification-report deserialization for offline review finalization. ### Security diff --git a/crates/conceptweave-zotero/src/lib.rs b/crates/conceptweave-zotero/src/lib.rs index a11258ec..b8d68e3e 100644 --- a/crates/conceptweave-zotero/src/lib.rs +++ b/crates/conceptweave-zotero/src/lib.rs @@ -145,18 +145,18 @@ pub enum AbstentionReason { } /// Evidence for a deterministic proposed disposition. -#[derive(Debug, Serialize)] +#[derive(Debug, Deserialize, Serialize)] pub struct ClassificationEvidence { /// Metadata fields whose values matched. - pub fields: Vec<&'static str>, + pub fields: Vec, /// Exact snapshot values for matched fields, retained only in the local report. - pub field_values: BTreeMap<&'static str, String>, + pub field_values: BTreeMap, /// Rule phrases found in those fields. - pub matched_phrases: Vec<&'static str>, + pub matched_phrases: Vec, } /// A single top-level bibliographic classification proposal. -#[derive(Debug, Serialize)] +#[derive(Debug, Deserialize, Serialize)] pub struct ClassifiedItem { /// Stable Zotero item key. pub item_key: String, @@ -1044,7 +1044,7 @@ impl fmt::Display for WritePlanError { impl std::error::Error for WritePlanError {} /// Complete local classification report for one immutable library version. -#[derive(Debug, Serialize)] +#[derive(Debug, Deserialize, Serialize)] pub struct ClassificationReport { /// Zotero desktop version that served the snapshot. pub zotero_version: String, @@ -1057,7 +1057,7 @@ pub struct ClassificationReport { /// Library version shared by every fetched page. pub library_version: u64, /// Rule revision used for all proposals. - pub rule_revision: &'static str, + pub rule_revision: String, /// Number of items read, including child notes and attachments. pub observed_item_count: usize, /// Complete item-revision identity of every observed record. @@ -1086,7 +1086,7 @@ pub struct ClassificationReport { } /// Aggregate-only evidence that a successful report covers its input and proposals. -#[derive(Debug, Clone, PartialEq, Eq, Serialize)] +#[derive(Debug, Clone, PartialEq, Eq, Deserialize, Serialize)] pub struct ClassificationAudit { /// Records captured from the immutable snapshot. pub snapshot_item_count: usize, @@ -1165,6 +1165,7 @@ pub fn build_steward_review_worksheet( if report.rule_revision.trim().is_empty() || report.snapshot_digest.trim().is_empty() { return Err(WorksheetError::InvalidReport); } + let mut decisions = Vec::with_capacity(report.classified_items.len()); for item in &report.classified_items { if (item.proposed_disposition == Disposition::NeedsStewardReview) @@ -1184,7 +1185,7 @@ pub fn build_steward_review_worksheet( Ok(StewardReviewWorksheet { library_version: report.library_version, - rule_revision: report.rule_revision.into(), + rule_revision: report.rule_revision.clone(), snapshot_digest: report.snapshot_digest.clone(), proposal_digest: classification_proposal_digest(report), snapshot_items: report.snapshot_items.clone(), @@ -1227,6 +1228,8 @@ pub struct SnapshotItemRevision { pub item_key: String, /// Item revision observed during review. pub item_version: u64, + /// Parent item key for child records; absent for top-level records. + pub parent_item_key: Option, } /// Governance receipt binding a steward approval to exact input and proposals. @@ -1465,8 +1468,15 @@ pub fn validate_classification_report( for item in &report.snapshot_items { if item.item_key.trim().is_empty() || item.item_version > report.library_version + || item + .parent_item_key + .as_ref() + .is_some_and(|key| key.trim().is_empty()) || remaining_items - .insert(item.item_key.as_str(), item.item_version) + .insert( + item.item_key.as_str(), + (item.item_version, item.parent_item_key.as_deref()), + ) .is_some() { return Err(invalid); @@ -1483,16 +1493,18 @@ pub fn validate_classification_report( item.item_type.as_str(), "attachment" | "note" | "annotation" ) - || remaining_items.remove(item.item_key.as_str()) != Some(item.item_version) + || remaining_items.remove(item.item_key.as_str()) != Some((item.item_version, None)) || reported_children != actual_children { return Err(invalid); } } for item in &report.unclassified_items { + let parent_key = + (!item.data.parent_item.is_empty()).then_some(item.data.parent_item.as_str()); if item.data.item_type.trim().is_empty() || is_bibliographic(item) - || remaining_items.remove(item.key.as_str()) != Some(item.version) + || remaining_items.remove(item.key.as_str()) != Some((item.version, parent_key)) { return Err(invalid); } @@ -1658,10 +1670,10 @@ where return Err(DuplicateReviewError::SnapshotMismatch); } validate_classification_report(report).map_err(|_| DuplicateReviewError::InvalidReview)?; - let item_revisions = report + let item_coordinates = report .snapshot_items .iter() - .map(|item| (item.item_key.as_str(), item.item_version)) + .map(|item| (item.item_key.as_str(), item)) .collect::>(); let candidates = report .duplicate_candidates @@ -1731,12 +1743,10 @@ where let source_items = component_keys .iter() .map(|item_key| { - item_revisions + item_coordinates .get(item_key.as_str()) - .map(|item_version| SnapshotItemRevision { - item_key: (*item_key).clone(), - item_version: *item_version, - }) + .filter(|item| item.parent_item_key.is_none()) + .map(|item| (*item).clone()) .ok_or(DuplicateReviewError::InvalidReview) }) .collect::, _>>()?; @@ -1762,6 +1772,7 @@ where 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)) @@ -2655,6 +2666,8 @@ pub fn classify_snapshot( .map(|item| SnapshotItemRevision { item_key: item.key.clone(), item_version: item.version, + parent_item_key: (!item.data.parent_item.is_empty()) + .then(|| item.data.parent_item.clone()), }) .collect(); let snapshot_records: Vec<_> = items @@ -2692,7 +2705,7 @@ pub fn classify_snapshot( schema_version: None, server_id, library_version, - rule_revision: RULE_REVISION, + rule_revision: RULE_REVISION.to_owned(), observed_item_count, snapshot_items, snapshot_digest, @@ -2933,9 +2946,12 @@ fn classify_item(item: &ZoteroItem, child_item_keys: Vec) -> ClassifiedI proposed_disposition, abstention_reason, evidence: ClassificationEvidence { - fields: matched_fields.into_iter().collect(), - field_values, - matched_phrases: matched_phrases.into_iter().collect(), + fields: matched_fields.into_iter().map(str::to_owned).collect(), + field_values: field_values + .into_iter() + .map(|(field, value)| (field.to_owned(), value)) + .collect(), + matched_phrases: matched_phrases.into_iter().map(str::to_owned).collect(), }, child_item_keys, model_receipt: None, @@ -3179,7 +3195,7 @@ mod tests { server_id: report.server_id.clone(), zotero_version: report.zotero_version.clone(), library_version: report.library_version, - rule_revision: report.rule_revision.into(), + rule_revision: report.rule_revision.clone(), snapshot_digest: report.snapshot_digest.clone(), proposal_digest: classification_proposal_digest(&report), snapshot_items: report.snapshot_items.clone(), diff --git a/crates/conceptweave-zotero/src/tests/authenticated_transport.rs b/crates/conceptweave-zotero/src/tests/authenticated_transport.rs index d17be9ed..369b4fbe 100644 --- a/crates/conceptweave-zotero/src/tests/authenticated_transport.rs +++ b/crates/conceptweave-zotero/src/tests/authenticated_transport.rs @@ -21,7 +21,7 @@ fn failed_http_write_with_matching_observation_remains_indeterminate() { server_id: report.server_id.clone(), zotero_version: report.zotero_version.clone(), library_version: report.library_version, - rule_revision: report.rule_revision.into(), + rule_revision: report.rule_revision.clone(), snapshot_digest: report.snapshot_digest.clone(), proposal_digest: classification_proposal_digest(&report), snapshot_items: report.snapshot_items.clone(), diff --git a/crates/conceptweave-zotero/tests/classification_report_roundtrip.rs b/crates/conceptweave-zotero/tests/classification_report_roundtrip.rs new file mode 100644 index 00000000..ee0b7419 --- /dev/null +++ b/crates/conceptweave-zotero/tests/classification_report_roundtrip.rs @@ -0,0 +1,313 @@ +use conceptweave_zotero::{ + ClassificationReport, Disposition, EvaluationError, GoldenSetApproval, ItemData, + ReviewedGoldenSet, WorksheetError, ZoteroItem, build_steward_review_worksheet, + classification_proposal_digest, classify_snapshot, evaluate_complete_reviewed_classification, + reviewed_golden_set_from_worksheet, +}; + +#[test] +fn owner_only_report_roundtrip_preserves_the_review_workload() { + let mut item = ZoteroItem { + source_record: None, + key: "ITEM".into(), + version: 7, + data: ItemData { + item_type: "journalArticle".into(), + title: "ontology alignment".into(), + abstract_note: "review context".into(), + doi: "10.1000/example".into(), + parent_item: String::new(), + collections: vec!["COLLECTION".into()], + tags: vec![], + }, + }; + item.data.tags.push(conceptweave_zotero::ItemTag { + tag: "ontology".into(), + tag_type: Some(1), + }); + let original = classify_snapshot("9.0.6".into(), None, 42, vec![item]); + let serialized = serde_json::to_vec(&original).unwrap(); + + let restored: ClassificationReport = serde_json::from_slice(&serialized).unwrap(); + + assert_eq!(restored.library_version, original.library_version); + assert_eq!(restored.rule_revision, original.rule_revision); + assert_eq!(restored.snapshot_digest, original.snapshot_digest); + assert_eq!( + build_steward_review_worksheet(&restored).unwrap(), + build_steward_review_worksheet(&original).unwrap() + ); + assert_eq!( + classification_proposal_digest(&restored), + classification_proposal_digest(&original) + ); + + let mut worksheet = build_steward_review_worksheet(&original).unwrap(); + worksheet.decisions[0].reviewed_disposition = Some(Disposition::AlignmentVersioning); + let approval = GoldenSetApproval { + receipt_id: "synthetic-roundtrip-receipt".into(), + reviewer_subject: "synthetic-steward".into(), + library_version: original.library_version, + rule_revision: original.rule_revision.clone(), + snapshot_digest: original.snapshot_digest.clone(), + proposal_digest: classification_proposal_digest(&original), + snapshot_items: original.snapshot_items.clone(), + }; + let golden = reviewed_golden_set_from_worksheet(&original, &worksheet, approval).unwrap(); + let restored_golden: ReviewedGoldenSet = + serde_json::from_slice(&serde_json::to_vec(&golden).unwrap()).unwrap(); + assert_eq!( + evaluate_complete_reviewed_classification(&restored, &restored_golden, |candidate| { + candidate == &golden + }) + .unwrap() + .correct_count, + 1 + ); + + for alter_title in [false, true] { + let mut changed_json = serde_json::to_value(&original).unwrap(); + if alter_title { + changed_json["classified_items"][0]["title"] = "changed after review".into(); + } else { + changed_json["classified_items"][0]["evidence"]["field_values"] = serde_json::json!({}); + } + let changed_report: ClassificationReport = serde_json::from_value(changed_json).unwrap(); + assert_eq!(changed_report.snapshot_digest, original.snapshot_digest); + assert_ne!( + build_steward_review_worksheet(&changed_report).unwrap(), + build_steward_review_worksheet(&original).unwrap() + ); + assert_eq!( + reviewed_golden_set_from_worksheet( + &changed_report, + &worksheet, + restored_golden.approval.clone() + ), + Err(EvaluationError::SnapshotMismatch) + ); + let verifier_calls = std::cell::Cell::new(0); + assert_eq!( + evaluate_complete_reviewed_classification( + &changed_report, + &restored_golden, + |candidate| { + verifier_calls.set(verifier_calls.get() + 1); + candidate == &golden + } + ), + Err(EvaluationError::SnapshotMismatch) + ); + assert_eq!(verifier_calls.get(), 0); + } +} + +#[test] +fn shared_report_validation_binds_every_retained_parent_coordinate() { + for parent_key in ["A", "missing", "source", ""] { + let items = [("A", "book", ""), ("source", "attachment", parent_key)] + .into_iter() + .map(|(key, item_type, parent)| { + serde_json::from_value::(serde_json::json!({ + "key": key, "version": 1, + "data": {"itemType": item_type, "parentItem": parent, "title": "synthetic"} + })) + .unwrap() + }) + .collect(); + let mut report = classify_snapshot("9.0.6".into(), None, 42, items); + assert!(conceptweave_zotero::validate_classification_report(&report).is_ok()); + report + .snapshot_items + .iter_mut() + .find(|item| item.item_key == "source") + .unwrap() + .parent_item_key = Some("different-parent".into()); + assert_eq!( + conceptweave_zotero::validate_classification_report(&report), + Err(EvaluationError::InvalidReview) + ); + } +} + +#[test] +fn pending_metadata_roundtrip_preserves_review_identity_not_raw_capture() { + for parent_key in ["missing", "source", ""] { + let item: ZoteroItem = serde_json::from_value(serde_json::json!({ + "key": "source", "version": 1, "unknown_provider_field": "raw-only-sentinel", + "data": {"itemType": "attachment", "parentItem": parent_key, "title": "synthetic"} + })) + .unwrap(); + let original = classify_snapshot("9.0.6".into(), None, 42, vec![item]); + let bytes = serde_json::to_vec(&original).unwrap(); + assert!( + !String::from_utf8(bytes.clone()) + .unwrap() + .contains("raw-only-sentinel") + ); + let restored: ClassificationReport = serde_json::from_slice(&bytes).unwrap(); + let restored_again: ClassificationReport = + serde_json::from_slice(&serde_json::to_vec(&restored).unwrap()).unwrap(); + assert_eq!(serde_json::to_vec(&restored_again).unwrap(), bytes); + assert_eq!( + classification_proposal_digest(&original), + classification_proposal_digest(&restored_again) + ); + assert_eq!( + build_steward_review_worksheet(&original).unwrap(), + build_steward_review_worksheet(&restored_again).unwrap() + ); + } +} + +#[test] +fn restored_report_rejects_child_provenance_outside_the_bound_snapshot() { + let item = ZoteroItem { + source_record: None, + key: "ITEM".into(), + version: 7, + data: ItemData { + item_type: "journalArticle".into(), + title: "ontology alignment".into(), + abstract_note: "review context".into(), + doi: "10.1000/example".into(), + parent_item: String::new(), + collections: vec!["COLLECTION".into()], + tags: vec![], + }, + }; + let original = classify_snapshot("9.0.6".into(), None, 42, vec![item]); + let serialized = serde_json::to_vec(&original).unwrap(); + let mut restored: ClassificationReport = serde_json::from_slice(&serialized).unwrap(); + + restored.classified_items[0].child_item_keys = vec!["UNKNOWN_CHILD".into()]; + + assert_eq!( + build_steward_review_worksheet(&restored), + Err(WorksheetError::InvalidReport) + ); + + let mut blank_parent: ClassificationReport = serde_json::from_slice(&serialized).unwrap(); + blank_parent.snapshot_items[0].parent_item_key = Some(" ".into()); + assert_eq!( + build_steward_review_worksheet(&blank_parent), + Err(WorksheetError::InvalidReport) + ); +} + +#[test] +fn restored_report_rejects_classified_or_reused_child_provenance() { + let bibliographic = |key: &str| ZoteroItem { + source_record: None, + key: key.into(), + version: 7, + data: ItemData { + item_type: "journalArticle".into(), + title: "ontology alignment".into(), + abstract_note: String::new(), + doi: String::new(), + parent_item: String::new(), + collections: vec![], + tags: vec![], + }, + }; + let child = ZoteroItem { + source_record: None, + key: "CHILD".into(), + version: 3, + data: ItemData { + item_type: "note".into(), + title: String::new(), + abstract_note: String::new(), + doi: String::new(), + parent_item: "PARENT_A".into(), + collections: vec![], + tags: vec![], + }, + }; + let report = classify_snapshot( + "9.0.6".into(), + None, + 42, + vec![bibliographic("PARENT_A"), bibliographic("PARENT_B"), child], + ); + let serialized = serde_json::to_vec(&report).unwrap(); + + let mut classified_child: ClassificationReport = serde_json::from_slice(&serialized).unwrap(); + classified_child.classified_items[0].child_item_keys = vec!["PARENT_A".into()]; + assert_eq!( + build_steward_review_worksheet(&classified_child), + Err(WorksheetError::InvalidReport) + ); + + let mut reused_child: ClassificationReport = serde_json::from_slice(&serialized).unwrap(); + reused_child.classified_items[1].child_item_keys = vec!["CHILD".into()]; + assert_eq!( + build_steward_review_worksheet(&reused_child), + Err(WorksheetError::InvalidReport) + ); + + let mut omitted_child: ClassificationReport = serde_json::from_slice(&serialized).unwrap(); + omitted_child.classified_items[0].child_item_keys.clear(); + assert_eq!( + build_steward_review_worksheet(&omitted_child), + Err(WorksheetError::InvalidReport) + ); + + let mut duplicate_child: ClassificationReport = serde_json::from_slice(&serialized).unwrap(); + duplicate_child.classified_items[0] + .child_item_keys + .push("CHILD".into()); + assert_eq!( + build_steward_review_worksheet(&duplicate_child), + Err(WorksheetError::InvalidReport) + ); + + let mut orphaned_child: ClassificationReport = serde_json::from_slice(&serialized).unwrap(); + orphaned_child.classified_items[0].child_item_keys.clear(); + orphaned_child + .snapshot_items + .iter_mut() + .find(|item| item.item_key == "CHILD") + .unwrap() + .parent_item_key = Some("UNCLASSIFIED_PARENT".into()); + assert_eq!( + build_steward_review_worksheet(&orphaned_child), + Err(WorksheetError::InvalidReport) + ); +} + +#[test] +fn restored_report_accepts_snapshot_bound_nested_child_provenance() { + let nested_item = |key: &str, item_type: &str, parent_item: &str| ZoteroItem { + source_record: None, + key: key.into(), + version: 7, + data: ItemData { + item_type: item_type.into(), + title: if key == "BOOK" { + "ontology alignment" + } else { + "" + } + .into(), + abstract_note: String::new(), + doi: String::new(), + parent_item: parent_item.into(), + collections: vec![], + tags: vec![], + }, + }; + let report = classify_snapshot( + "9.0.6".into(), + None, + 42, + vec![ + nested_item("BOOK", "book", ""), + nested_item("ATTACHMENT", "attachment", "BOOK"), + nested_item("ANNOTATION", "annotation", "ATTACHMENT"), + ], + ); + + assert!(build_steward_review_worksheet(&report).is_ok()); +} diff --git a/crates/conceptweave-zotero/tests/classification_rollback.rs b/crates/conceptweave-zotero/tests/classification_rollback.rs index 4aed44d2..6e57b478 100644 --- a/crates/conceptweave-zotero/tests/classification_rollback.rs +++ b/crates/conceptweave-zotero/tests/classification_rollback.rs @@ -54,7 +54,7 @@ fn plan() -> conceptweave_zotero::ClassificationWritePlan { server_id: report.server_id.clone(), zotero_version: report.zotero_version.clone(), library_version: report.library_version, - rule_revision: report.rule_revision.into(), + rule_revision: report.rule_revision.clone(), snapshot_digest: report.snapshot_digest.clone(), proposal_digest: conceptweave_zotero::classification_proposal_digest(&report), snapshot_items: report.snapshot_items.clone(), diff --git a/crates/conceptweave-zotero/tests/classification_write_plan.rs b/crates/conceptweave-zotero/tests/classification_write_plan.rs index 86ac1ec4..6bb042dc 100644 --- a/crates/conceptweave-zotero/tests/classification_write_plan.rs +++ b/crates/conceptweave-zotero/tests/classification_write_plan.rs @@ -450,7 +450,7 @@ fn reviewed(report: &conceptweave_zotero::ClassificationReport) -> ReviewedClass server_id: report.server_id.clone(), zotero_version: report.zotero_version.clone(), library_version: report.library_version, - rule_revision: report.rule_revision.into(), + rule_revision: report.rule_revision.clone(), snapshot_digest: report.snapshot_digest.clone(), proposal_digest: classification_proposal_digest(report), snapshot_items: report.snapshot_items.clone(), @@ -680,7 +680,7 @@ fn write_plan_fails_closed_for_untrusted_stale_or_unsafe_changes() { server_id: None, zotero_version: no_server.zotero_version.clone(), library_version: no_server.library_version, - rule_revision: no_server.rule_revision.into(), + rule_revision: no_server.rule_revision.clone(), snapshot_digest: no_server.snapshot_digest.clone(), proposal_digest: classification_proposal_digest(&no_server), snapshot_items: no_server.snapshot_items.clone(), diff --git a/crates/conceptweave-zotero/tests/classification_write_receipt_contract.rs b/crates/conceptweave-zotero/tests/classification_write_receipt_contract.rs index 7b9338c8..eb4892f4 100644 --- a/crates/conceptweave-zotero/tests/classification_write_receipt_contract.rs +++ b/crates/conceptweave-zotero/tests/classification_write_receipt_contract.rs @@ -56,7 +56,7 @@ fn reviewed(report: &conceptweave_zotero::ClassificationReport) -> ReviewedClass server_id: report.server_id.clone(), zotero_version: report.zotero_version.clone(), library_version: report.library_version, - rule_revision: report.rule_revision.into(), + rule_revision: report.rule_revision.clone(), snapshot_digest: report.snapshot_digest.clone(), proposal_digest: conceptweave_zotero::classification_proposal_digest(report), snapshot_items: report.snapshot_items.clone(), diff --git a/crates/conceptweave-zotero/tests/duplicate_merge_review_components.rs b/crates/conceptweave-zotero/tests/duplicate_merge_review_components.rs index 21f95876..526603ad 100644 --- a/crates/conceptweave-zotero/tests/duplicate_merge_review_components.rs +++ b/crates/conceptweave-zotero/tests/duplicate_merge_review_components.rs @@ -39,7 +39,7 @@ fn transitive_duplicate_component_accepts_one_component_level_canonical_key() { review_id: "review-transitive-component".into(), authority_receipt: "authority-transitive-component".into(), library_version: report.library_version, - rule_revision: report.rule_revision.into(), + rule_revision: report.rule_revision.clone(), snapshot_digest: report.snapshot_digest.clone(), proposal_digest: conceptweave_zotero::classification_proposal_digest(&report), snapshot_items: report.snapshot_items.clone(), @@ -96,7 +96,7 @@ fn duplicate_review_rejects_blank_snapshot_item_identity() { review_id: "review-blank-key".into(), authority_receipt: "authority-blank-key".into(), library_version: report.library_version, - rule_revision: report.rule_revision.into(), + rule_revision: report.rule_revision.clone(), snapshot_digest: report.snapshot_digest.clone(), proposal_digest: conceptweave_zotero::classification_proposal_digest(&report), snapshot_items: report.snapshot_items.clone(), @@ -138,7 +138,7 @@ fn duplicate_review_binds_every_item_revision_to_the_reviewed_snapshot() { review_id: "review-revisions".into(), authority_receipt: "authority-revisions".into(), library_version: report.library_version, - rule_revision: report.rule_revision.into(), + rule_revision: report.rule_revision.clone(), snapshot_digest: report.snapshot_digest.clone(), proposal_digest: conceptweave_zotero::classification_proposal_digest(&report), snapshot_items: report.snapshot_items.clone(), diff --git a/crates/conceptweave-zotero/tests/duplicate_merge_review_manifest.rs b/crates/conceptweave-zotero/tests/duplicate_merge_review_manifest.rs index ed085596..e4863fa9 100644 --- a/crates/conceptweave-zotero/tests/duplicate_merge_review_manifest.rs +++ b/crates/conceptweave-zotero/tests/duplicate_merge_review_manifest.rs @@ -2,6 +2,7 @@ use conceptweave_zotero::{ Disposition, DuplicateMergeDecision, DuplicateReviewError, ItemData, ReviewedDuplicateMergeSet, ZoteroItem, build_duplicate_merge_review_manifest, classify_snapshot, }; +use std::cell::Cell; fn item(key: &str, version: u64, title: &str, doi: &str) -> ZoteroItem { ZoteroItem { @@ -49,7 +50,7 @@ fn reviewed(report: &conceptweave_zotero::ClassificationReport) -> ReviewedDupli review_id: "review-duplicate-1".into(), authority_receipt: "authority-receipt-1".into(), library_version: report.library_version, - rule_revision: report.rule_revision.into(), + rule_revision: report.rule_revision.clone(), snapshot_digest: report.snapshot_digest.clone(), proposal_digest: conceptweave_zotero::classification_proposal_digest(report), snapshot_items: report.snapshot_items.clone(), @@ -267,6 +268,30 @@ fn duplicate_review_contract_fails_closed() { } } +#[test] +fn duplicate_review_rejects_child_sources_before_approval_verification() { + let mut report = report(); + report + .snapshot_items + .push(conceptweave_zotero::SnapshotItemRevision { + item_key: "CHILD".into(), + item_version: 3, + parent_item_key: Some("A".into()), + }); + report.duplicate_candidates[0].item_keys[1] = "CHILD".into(); + let reviewed = reviewed(&report); + let verifier_calls = Cell::new(0); + + assert_eq!( + build_duplicate_merge_review_manifest(&report, &reviewed, |_| { + verifier_calls.set(verifier_calls.get() + 1); + true + }), + Err(DuplicateReviewError::InvalidReview) + ); + assert_eq!(verifier_calls.get(), 0); +} + #[test] fn overlapping_duplicate_groups_require_one_consistent_canonical_choice() { let report = classify_snapshot( @@ -284,7 +309,7 @@ fn overlapping_duplicate_groups_require_one_consistent_canonical_choice() { review_id: "review-overlap".into(), authority_receipt: "authority-overlap".into(), library_version: report.library_version, - rule_revision: report.rule_revision.into(), + rule_revision: report.rule_revision.clone(), snapshot_digest: report.snapshot_digest.clone(), proposal_digest: conceptweave_zotero::classification_proposal_digest(&report), snapshot_items: report.snapshot_items.clone(), @@ -328,7 +353,7 @@ fn duplicate_review_rejects_ambiguous_snapshot_key_revisions() { review_id: "review-duplicate-key".into(), authority_receipt: "authority-duplicate-key".into(), library_version: report.library_version, - rule_revision: report.rule_revision.into(), + rule_revision: report.rule_revision.clone(), snapshot_digest: report.snapshot_digest.clone(), proposal_digest: conceptweave_zotero::classification_proposal_digest(&report), snapshot_items: report.snapshot_items.clone(), diff --git a/crates/conceptweave-zotero/tests/golden_set_evaluation.rs b/crates/conceptweave-zotero/tests/golden_set_evaluation.rs index ccd6bf36..94174f72 100644 --- a/crates/conceptweave-zotero/tests/golden_set_evaluation.rs +++ b/crates/conceptweave-zotero/tests/golden_set_evaluation.rs @@ -177,6 +177,7 @@ fn golden(labels: Vec) -> ReviewedGoldenSet { .map(|item_key| SnapshotItemRevision { item_key: item_key.into(), item_version: 1, + parent_item_key: None, }) .collect(), }, @@ -288,7 +289,7 @@ fn reviewed_golden_set_rejects_stale_unknown_and_duplicate_labels() { evaluate_reviewed_golden_set(&report, &stale, verify_synthetic_approval), Err(EvaluationError::SnapshotMismatch) ); - stale.approval.rule_revision = report.rule_revision.into(); + stale.approval.rule_revision = report.rule_revision.clone(); stale.approval.snapshot_items[0].item_version += 1; assert_eq!( evaluate_reviewed_golden_set(&report, &stale, verify_synthetic_approval), diff --git a/crates/conceptweave-zotero/tests/golden_set_integrity_contract.rs b/crates/conceptweave-zotero/tests/golden_set_integrity_contract.rs index f14dce4e..e63fe6b4 100644 --- a/crates/conceptweave-zotero/tests/golden_set_integrity_contract.rs +++ b/crates/conceptweave-zotero/tests/golden_set_integrity_contract.rs @@ -149,7 +149,7 @@ fn approval( receipt_id: "approved-review".into(), reviewer_subject: "synthetic-steward".into(), library_version: report.library_version, - rule_revision: report.rule_revision.into(), + rule_revision: report.rule_revision.clone(), snapshot_digest: classification_snapshot_digest(report), proposal_digest: classification_proposal_digest(report), snapshot_items, @@ -174,10 +174,12 @@ fn reviewed_snapshot_binding_includes_linked_child_revisions() { SnapshotItemRevision { item_key: "PARENT".into(), item_version: 7, + parent_item_key: None, }, SnapshotItemRevision { item_key: "NOTE1".into(), item_version: 3, + parent_item_key: Some("PARENT".into()), }, ], ), @@ -204,6 +206,7 @@ fn verified_snapshot_receipt_cannot_authorize_mutated_steward_labels() { vec![SnapshotItemRevision { item_key: "A".into(), item_version: 1, + parent_item_key: None, }], ), labels: vec![GoldenLabel::new("A", Disposition::AlignmentVersioning)], @@ -237,10 +240,12 @@ fn duplicate_zotero_keys_fail_closed_even_when_item_revisions_differ() { SnapshotItemRevision { item_key: "A".into(), item_version: 1, + parent_item_key: None, }, SnapshotItemRevision { item_key: "A".into(), item_version: 2, + parent_item_key: None, }, ], ), @@ -547,6 +552,12 @@ fn source_metadata_mutations_invalidate_the_original_approval_before_verificatio "type" => source.data.item_type = "attachment".into(), "parent" => { source.data.parent_item = "C".into(); + report + .snapshot_items + .iter_mut() + .find(|item| item.item_key == source.key) + .unwrap() + .parent_item_key = Some("C".into()); report.pending_source_item_keys = vec!["T".into()]; } _ => unreachable!(), diff --git a/crates/conceptweave-zotero/tests/snapshot_content_binding.rs b/crates/conceptweave-zotero/tests/snapshot_content_binding.rs index fbd35c18..3471c8c1 100644 --- a/crates/conceptweave-zotero/tests/snapshot_content_binding.rs +++ b/crates/conceptweave-zotero/tests/snapshot_content_binding.rs @@ -36,6 +36,7 @@ fn golden_approval_rejects_same_revision_coordinates_with_changed_snapshot_conte snapshot_items: vec![SnapshotItemRevision { item_key: "A".into(), item_version: 1, + parent_item_key: None, }], }, labels: vec![GoldenLabel::new("A", Disposition::Generation)], diff --git a/crates/conceptweave-zotero/tests/steward_review_context.rs b/crates/conceptweave-zotero/tests/steward_review_context.rs index 73c27a94..0edcc30e 100644 --- a/crates/conceptweave-zotero/tests/steward_review_context.rs +++ b/crates/conceptweave-zotero/tests/steward_review_context.rs @@ -134,7 +134,7 @@ fn changed_review_context_invalidates_prior_approval_before_verification() { receipt_id: "synthetic-receipt".into(), reviewer_subject: "synthetic-steward".into(), library_version: report.library_version, - rule_revision: report.rule_revision.into(), + rule_revision: report.rule_revision.clone(), snapshot_digest: report.snapshot_digest.clone(), proposal_digest: classification_proposal_digest(&report), snapshot_items: report.snapshot_items.clone(), diff --git a/crates/conceptweave-zotero/tests/steward_review_finalization.rs b/crates/conceptweave-zotero/tests/steward_review_finalization.rs index 27e6a057..46c0aa05 100644 --- a/crates/conceptweave-zotero/tests/steward_review_finalization.rs +++ b/crates/conceptweave-zotero/tests/steward_review_finalization.rs @@ -84,7 +84,7 @@ fn finalization_rejects_each_invalid_identity_coordinate() { let worksheet = complete_worksheet(); let mut invalid_report = report(); - invalid_report.rule_revision = ""; + invalid_report.rule_revision.clear(); assert_eq!( reviewed_golden_set_from_worksheet( &invalid_report, diff --git a/crates/conceptweave-zotero/tests/steward_review_worksheet.rs b/crates/conceptweave-zotero/tests/steward_review_worksheet.rs index 143edfa7..fd14738c 100644 --- a/crates/conceptweave-zotero/tests/steward_review_worksheet.rs +++ b/crates/conceptweave-zotero/tests/steward_review_worksheet.rs @@ -153,7 +153,7 @@ fn worksheet_is_snapshot_bound_complete_and_contains_no_bibliographic_text() { #[test] fn worksheet_rejects_each_inconsistent_report_coordinate() { let mut invalid = report(); - invalid.rule_revision = ""; + invalid.rule_revision.clear(); assert_eq!( build_steward_review_worksheet(&invalid), Err(WorksheetError::InvalidReport) diff --git a/docs/TRD.md b/docs/TRD.md index 8df75fb6..5b6c4bdc 100644 --- a/docs/TRD.md +++ b/docs/TRD.md @@ -97,6 +97,7 @@ 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. Its nonempty abstract is retained exactly once in the local report: an abstract that triggered conflicting rules remains in matched evidence, while other abstention abstracts use the review-only field. Non-abstained items omit that field. 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. +Duplicate sources retain full item key/version/parent coordinates and must be top-level records; all parent checks finish before governance. 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. @@ -113,6 +114,7 @@ A successful classification report carries an `audit_summary` whose snapshot, bi The review worksheet is a deterministic item-key-ordered projection of the report. It binds library/rule revisions, raw-snapshot digest, required `proposal_digest` from the existing v2 scope hash, complete item coordinates, proposal, abstention reason, and one initially empty decision per bibliographic item. Construction reuses `validate_classification_report`, mapped to `WorksheetError::InvalidReport`, before worksheet-specific nonblank identity and abstention checks. This avoids a second drifting audit implementation while admitting structurally valid pending source evidence. It deliberately omits bibliographic text and matched evidence; stewards consult the owner-only report by item key. Missing proposal binding fails deserialization. Subsequent progress, application and finalization owners must compare this field to the recomputed report binding; present-but-blank or rewritten values are not authority. Legacy worksheets must be regenerated, and independent approval remains separate. The finalization function consumes the filled worksheet plus a governance approval receipt. It rejects blank authority metadata, missing or mismatched proposal bindings, coordinate drift, duplicate or unknown decision keys, missing decisions, abstention as approved truth, and tampered proposal/reason pairs. It recomputes the complete current proposal digest, including title and evidence fields omitted from the worksheet, and compares it with the supplied approval without replacing that approval. Its output reuses the existing reviewed-golden-set evaluator; finalization itself does not verify external authority. It also rejects a blank worksheet `proposal_digest` as `InvalidReview` and compares that field with the freshly built expected worksheet as `SnapshotMismatch`, retaining existing cardinality and approval error precedence. The converter does not accept an updated receipt as proof that old decisions reviewed changed content. A caller can construct self-consistent unverified data, so the evaluator still authenticates the entire reviewed set against independent evidence. Pending sources are admitted for preparation but rejected by complete evaluation before governance is contacted. Later extracted worksheet validators must preserve this comparison. +The owner-only report uses owned JSON values and supports lossless deserialization of its defined metadata projection, not the original provider record or full-text capture. Unknown provider fields and raw capture bytes are not serialized; retain captures separately. Repeated serialize/deserialize roundtrips preserve report bytes, the report-derived worksheet and proposal digest, allowing offline finalization against the stored snapshot identity rather than another live Zotero read. Shared report validation binds every retained key, version and parent coordinate; classified sources are top-level, while valid unresolved orphan and cyclic metadata remains pending. Deserialization establishes structure, not approval: changing serialized title or evidence under an unchanged source digest must still fail both finalization and evaluation against the original receipt before external verification. The local report can contain titles, tags, matched metadata, and abstention abstracts. It is sensitive steward-review material, remains outside the repository, and is not a publication artifact. diff --git a/docs/adr/0006-zotero-research-intake.md b/docs/adr/0006-zotero-research-intake.md index 931021cb..6aa2f86a 100644 --- a/docs/adr/0006-zotero-research-intake.md +++ b/docs/adr/0006-zotero-research-intake.md @@ -23,12 +23,20 @@ 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. An abstention likewise retains its nonempty abstract so a steward can resolve unsupported or unmatched vocabulary from the same immutable report. If matched evidence already contains the abstract, the review-only field is omitted so sensitive text appears once; decided items also omit that extra copy. On supported Unix platforms, the sensitive local report is created with exact owner-only `0600` permissions after applying the process umask; other platforms fail closed. 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. +Duplicate candidates become canonical references only through externally verified steward decisions bound to the raw digest, complete item key/version/parent snapshot, and exact candidate membership. Duplicate sources must be observed top-level records, and local parent, component, and retained-key validation finishes before approval verification. Overlapping candidates form one connected component and must select one component-level canonical item. Every resulting operation preserves all component source coordinates 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 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. Sampled labels may measure classifier quality, but a full-reclassification completion result requires exactly one approved label for every classified bibliographic item. Cardinality, snapshot, key, disposition, and duplicate checks run before the external approval verifier so invalid local input cannot consume approval authority. Evaluation returns the verified revisions and opaque digest with aggregate integer evidence; Zotero keys, reviewer identity, and bibliographic text are omitted. Missing, incomplete, stale, content- or label-mismatched, unverified, unknown, duplicate, or invalid review identities fail closed at the applicable completion boundary. 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. -To make full review executable without copying sensitive text again, ConceptWeave derives a local worksheet from the validated report. The worksheet carries the exact snapshot binding, complete parent/child revision coordinates, deterministic proposal and abstention reason, plus one empty steward-decision slot per bibliographic item in item-key order. Titles, abstracts, tags, collections, and matched evidence remain only in the owner-only report. Invalid report identity or coverage fails worksheet construction. +To make full review executable without copying sensitive text again, ConceptWeave derives a local worksheet from the validated report. The worksheet carries the exact snapshot binding, complete item key/version/parent coordinates, deterministic proposal and abstention reason, plus one empty steward-decision slot per bibliographic item in item-key order. Restored artifacts must preserve each observed child-to-parent relation; a classified item cannot masquerade as a child, and an in-snapshot child cannot be reassigned to another parent. Titles, abstracts, tags, collections, and matched evidence remain only in the owner-only report. Invalid report identity or coverage fails worksheet construction. + +The report is a losslessly deserializable owner-only metadata projection, not a raw-source backup. Evidence field names, matched phrases, and the rule revision use owned values so an offline process can reconstruct the exact canonical worksheet from the saved report; it must not reread a mutable Zotero library to finalize an earlier review. + +### Proposed parent-coordinate continuity amendment — September 7 + +PR28 adds parent coordinates and offline report restoration on top of the repaired source-scope and worksheet contracts. Shared validation previously checked only key/version and could accept a changed retained parent; the worksheet's duplicate validation instead rejected legitimate pending orphan evidence. RED `97fc490` reproduces both failures. `1157b1d` compares key/version/parent once in the shared validator and removes the divergent worksheet checks. `e102894` removes the now-redundant evaluator guard and retains stale-approval rejection for structurally coherent metadata mutations. Classified records require no parent; retained unclassified metadata must match its explicit parent coordinate. Missing or cyclic ancestry remains pending, never automatically classified or approved. + +We rejected a second worksheet-only validator because sibling evaluation and duplicate admission would remain inconsistent. We rejected storing raw capture fields in this metadata artifact because it would broaden the private data surface and conflate full-text capture with review projection. Unit tests verify repeated serialization, proposal digest and worksheet identity while proving an unknown raw field stays excluded. The trade-off is intentional: reconstructed metadata cannot replace the original capture or independently recompute its raw-source digest. Later consumers must inherit this contract and keep capture verification and approval separate. This amendment remains Proposed pending protected review and release. Proposed worksheet source-scope amendment: in the context of continuing review across saved local worksheets, facing removed retained records or changed context at unchanged item revisions, we decided for the existing shared report validator and required v2 proposal digest, and against a duplicate worksheet audit or coordinate-only identity, to preserve complete source scope and distinguish changed review material, accepting a breaking serialized-field requirement and downstream comparison work. RED `900038e` reproduces omitted inventory and hidden pending keys; `5b54d06` removes the divergent checks in favor of the shared validator. RED `30aa091` reproduces equal worksheets for changed content and successful loading without binding; `97046c7` reuses the existing scope hash. Valid standalone, orphan and cyclic evidence still permits blank worksheet creation, because preparation is not completion. No source metadata is copied into a new artifact. diff --git a/docs/product-technical-gap-baseline.md b/docs/product-technical-gap-baseline.md index 864e14c3..a695640e 100644 --- a/docs/product-technical-gap-baseline.md +++ b/docs/product-technical-gap-baseline.md @@ -6,6 +6,14 @@ This file records code-current product and technical gaps. Exact PR/check/run co ## September 6 source inventory checkpoint +### September 7 PR28 metadata roundtrip continuity repair + +Normal merge `13b5529` preserves original PR28 `ba6b3dfc71cf89ed4c57b85da0dd9ca5f983efee` and PR27 `fd5ef23c1c23fa36ef106400e15c9438eaa5cd41`. RED `97fc490` compiled with four passing and two failing roundtrip tests: inconsistent retained parent coordinates passed shared admission, while valid pending orphan metadata failed worksheet restoration. `1157b1d` centralizes parent validation and removes divergent worksheet checks; `e102894` removes an implied evaluator guard and preserves coherent-mutation stale-approval rejection. Independent read-only review found no further production defect in the bounded repair. + +Source `e102894` passes 179 tests/26 suites including three doctests, strict all-target Clippy, warnings-denied rustdoc, formatting, CI contract and diff checks. The unchanged coverage gate passes 299/299 functions, 2,642/2,642 normalized regions and 470/470 normalized branches. Raw coverage remains 3,449/3,511 lines, 5,245/5,352 regions and 425/470 branches, not 100%. Logs: `/tmp/conceptweave-pr28-{parent-red,final-tests,clippy,rustdoc,coverage}.log`. Documentation amendment `29cc97a` distinguishes the restored metadata projection from raw source/full-text capture. Unknown provider fields stay excluded; no new capture mechanism or dependency was added. + +Root and later consumers have not yet inherited these repairs. Authentic decisions and independent approvals remain 0/3,715, with four unresolved standalone sources. Synthetic fixtures are not paper review evidence. Native Zotero Visual Inspection was retried but the Mac is locked; no new screen evidence exists. No real Zotero write, hosted GREEN, protected approval/merge or release is claimed. Preserve Draft and continue normal successor integration with fresh exact-head evidence. + ### September 7 PR27 finalization continuity repair Final source `da2556b` passes 172 tests/25 suites including three doctests, strict all-target Clippy, warnings-denied rustdoc, formatting, CI contract and diff checks. The unchanged pinned coverage gate passes 295/295 functions, 2,611/2,611 normalized regions and 468/468 normalized branches. Raw coverage remains 3,436/3,498 lines, 5,214/5,321 regions and 423/468 branches, not 100%. Logs: `/tmp/conceptweave-pr27-verified.log` and `/tmp/conceptweave-pr27-{clippy,rustdoc,coverage}-verified.log`. Both baseline and integration finished before their successors were edited; the final documentation commit receives its own full verification.