diff --git a/crates/conceptweave-zotero/src/lib.rs b/crates/conceptweave-zotero/src/lib.rs index 15ddf1d..fcdc0ee 100644 --- a/crates/conceptweave-zotero/src/lib.rs +++ b/crates/conceptweave-zotero/src/lib.rs @@ -1163,6 +1163,32 @@ pub struct StewardReviewProgress { pub complete: bool, } +/// One snapshot-bound steward decision supplied without rewriting a worksheet by hand. +#[derive(Debug, Clone, PartialEq, Eq, Deserialize, Serialize)] +pub struct StewardDecisionUpdate { + /// Stable Zotero item key of the reviewed bibliographic item. + pub item_key: String, + /// Exact item revision reviewed by the steward. + pub item_version: u64, + /// Human-reviewed truth label; abstention is not valid truth. + pub reviewed_disposition: Disposition, +} + +/// A bounded set of local steward decisions for one immutable classification snapshot. +#[derive(Debug, Clone, PartialEq, Eq, Deserialize, Serialize)] +pub struct StewardDecisionPatch { + /// Zotero library revision shared with the report and worksheet. + pub library_version: u64, + /// Classifier revision whose proposals were reviewed. + pub rule_revision: String, + /// Canonical digest of the complete raw snapshot. + pub snapshot_digest: String, + /// Required opaque identity of the proposals and retained metadata reviewed for this patch. + pub proposal_digest: String, + /// Unique item-revision decisions to apply atomically. + pub decisions: Vec, +} + /// A classification report cannot safely produce a review worksheet. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum WorksheetError { @@ -1253,6 +1279,63 @@ pub fn assess_steward_review_progress( }) } +/// Applies one snapshot-bound decision patch without overwriting conflicting review work. +pub fn apply_steward_decision_patch( + report: &ClassificationReport, + worksheet: &StewardReviewWorksheet, + patch: &StewardDecisionPatch, +) -> Result { + let expected = build_steward_review_worksheet(report)?; + validate_steward_review_worksheet_against(&expected, worksheet)?; + if worksheet + .decisions + .iter() + .any(|decision| decision.reviewed_disposition == Some(Disposition::NeedsStewardReview)) + || patch.library_version != expected.library_version + || patch.rule_revision != expected.rule_revision + || patch.snapshot_digest != expected.snapshot_digest + || patch.proposal_digest != expected.proposal_digest + || patch.decisions.is_empty() + { + return Err(WorksheetError::InvalidReport); + } + + let mut updates = BTreeMap::new(); + for decision in &patch.decisions { + if decision.item_key.trim().is_empty() + || decision.reviewed_disposition == Disposition::NeedsStewardReview + || updates + .insert( + decision.item_key.as_str(), + (decision.item_version, decision.reviewed_disposition), + ) + .is_some() + { + return Err(WorksheetError::InvalidReport); + } + } + + let mut updated = worksheet.clone(); + for decision in &mut updated.decisions { + let Some((item_version, reviewed_disposition)) = updates.remove(decision.item_key.as_str()) + else { + continue; + }; + if item_version != decision.item_version + || decision + .reviewed_disposition + .is_some_and(|existing| existing != reviewed_disposition) + { + return Err(WorksheetError::InvalidReport); + } + decision.reviewed_disposition = Some(reviewed_disposition); + } + if !updates.is_empty() { + return Err(WorksheetError::InvalidReport); + } + Ok(updated) +} + fn validate_steward_review_worksheet_against( expected: &StewardReviewWorksheet, worksheet: &StewardReviewWorksheet, diff --git a/crates/conceptweave-zotero/tests/steward_decision_patch.rs b/crates/conceptweave-zotero/tests/steward_decision_patch.rs new file mode 100644 index 0000000..ce927ea --- /dev/null +++ b/crates/conceptweave-zotero/tests/steward_decision_patch.rs @@ -0,0 +1,193 @@ +use conceptweave_zotero::{ + Disposition, ItemData, StewardDecisionPatch, StewardDecisionUpdate, WorksheetError, ZoteroItem, + apply_steward_decision_patch, build_steward_review_worksheet, classify_snapshot, +}; + +fn item(key: &str, title: &str) -> ZoteroItem { + ZoteroItem { + source_record: None, + key: key.into(), + version: 7, + data: ItemData { + item_type: "book".into(), + title: title.into(), + abstract_note: String::new(), + doi: String::new(), + parent_item: String::new(), + collections: vec![], + tags: vec![], + }, + } +} + +fn report() -> conceptweave_zotero::ClassificationReport { + classify_snapshot( + "9.0.6".into(), + None, + 42, + vec![item("A", "ontology learning"), item("B", "unmatched")], + ) +} + +fn patch(item_key: &str, item_version: u64, disposition: Disposition) -> StewardDecisionPatch { + let report = report(); + StewardDecisionPatch { + proposal_digest: conceptweave_zotero::classification_proposal_digest(&report), + library_version: report.library_version, + rule_revision: report.rule_revision, + snapshot_digest: report.snapshot_digest, + decisions: vec![StewardDecisionUpdate { + item_key: item_key.into(), + item_version, + reviewed_disposition: disposition, + }], + } +} + +#[test] +fn decision_patch_is_snapshot_bound_idempotent_and_non_overwriting() { + let report = report(); + let worksheet = build_steward_review_worksheet(&report).unwrap(); + let update = patch("A", 7, Disposition::AlignmentVersioning); + + let updated = apply_steward_decision_patch(&report, &worksheet, &update).unwrap(); + assert_eq!( + updated.decisions[0].reviewed_disposition, + Some(Disposition::AlignmentVersioning) + ); + assert_eq!( + apply_steward_decision_patch(&report, &updated, &update).unwrap(), + updated + ); + + let conflicting = patch("A", 7, Disposition::OutOfScope); + assert_eq!( + apply_steward_decision_patch(&report, &updated, &conflicting), + Err(WorksheetError::InvalidReport) + ); +} + +#[test] +fn stale_patch_cannot_use_a_fresh_worksheet_for_changed_content() { + let update = patch("A", 7, Disposition::AlignmentVersioning); + let mut report = report(); + report.classified_items[0] + .title + .push_str(" changed evidence"); + let worksheet = build_steward_review_worksheet(&report).unwrap(); + assert_eq!( + apply_steward_decision_patch(&report, &worksheet, &update), + Err(WorksheetError::InvalidReport) + ); +} + +#[test] +fn patch_deserialization_requires_content_binding() { + let mut value = serde_json::to_value(patch("A", 7, Disposition::AlignmentVersioning)).unwrap(); + value.as_object_mut().unwrap().remove("proposal_digest"); + assert!(serde_json::from_value::(value).is_err()); +} + +#[test] +fn late_invalid_update_preserves_the_original_worksheet() { + let report = report(); + let worksheet = build_steward_review_worksheet(&report).unwrap(); + let before = worksheet.clone(); + let mut update = patch("A", 7, Disposition::Generation); + update.decisions.push(StewardDecisionUpdate { + item_key: "UNKNOWN".into(), + item_version: 7, + reviewed_disposition: Disposition::OutOfScope, + }); + assert_eq!( + apply_steward_decision_patch(&report, &worksheet, &update), + Err(WorksheetError::InvalidReport) + ); + assert_eq!(worksheet, before); +} + +#[test] +fn decision_patch_rejects_invalid_identity_and_truth() { + let report = report(); + let worksheet = build_steward_review_worksheet(&report).unwrap(); + + let mut invalid_report = crate::report(); + invalid_report.rule_revision.clear(); + assert_eq!( + apply_steward_decision_patch( + &invalid_report, + &worksheet, + &patch("A", 7, Disposition::AlignmentVersioning) + ), + Err(WorksheetError::InvalidReport) + ); + + let mut invalid_worksheet = worksheet.clone(); + invalid_worksheet.decisions[0].item_version += 1; + assert_eq!( + apply_steward_decision_patch( + &report, + &invalid_worksheet, + &patch("A", 7, Disposition::AlignmentVersioning) + ), + Err(WorksheetError::InvalidReport) + ); + + let mut abstaining_worksheet = worksheet.clone(); + abstaining_worksheet.decisions[1].reviewed_disposition = Some(Disposition::NeedsStewardReview); + assert_eq!( + apply_steward_decision_patch( + &report, + &abstaining_worksheet, + &patch("A", 7, Disposition::AlignmentVersioning) + ), + Err(WorksheetError::InvalidReport) + ); + + let mut invalid = patch("A", 7, Disposition::AlignmentVersioning); + invalid.library_version += 1; + assert_eq!( + apply_steward_decision_patch(&report, &worksheet, &invalid), + Err(WorksheetError::InvalidReport) + ); + + let mut invalid = patch("A", 7, Disposition::AlignmentVersioning); + invalid.rule_revision.clear(); + assert_eq!( + apply_steward_decision_patch(&report, &worksheet, &invalid), + Err(WorksheetError::InvalidReport) + ); + + let mut invalid = patch("A", 7, Disposition::AlignmentVersioning); + invalid.snapshot_digest.clear(); + assert_eq!( + apply_steward_decision_patch(&report, &worksheet, &invalid), + Err(WorksheetError::InvalidReport) + ); + + for invalid in [ + patch(" ", 7, Disposition::OutOfScope), + patch("UNKNOWN", 7, Disposition::OutOfScope), + patch("A", 8, Disposition::OutOfScope), + patch("A", 7, Disposition::NeedsStewardReview), + ] { + assert_eq!( + apply_steward_decision_patch(&report, &worksheet, &invalid), + Err(WorksheetError::InvalidReport) + ); + } + + let mut duplicate = patch("A", 7, Disposition::AlignmentVersioning); + duplicate.decisions.push(duplicate.decisions[0].clone()); + assert_eq!( + apply_steward_decision_patch(&report, &worksheet, &duplicate), + Err(WorksheetError::InvalidReport) + ); + + let mut empty = patch("A", 7, Disposition::AlignmentVersioning); + empty.decisions.clear(); + assert_eq!( + apply_steward_decision_patch(&report, &worksheet, &empty), + Err(WorksheetError::InvalidReport) + ); +} diff --git a/docs/PRD.md b/docs/PRD.md index 90e0c65..4684708 100644 --- a/docs/PRD.md +++ b/docs/PRD.md @@ -92,6 +92,8 @@ Operators must be able to finalize the saved report, completed worksheet, and ap During the human review campaign, operators must be able to validate a partially completed worksheet against its original report and persist aggregate progress without an approval receipt. Progress binds current proposal and retained metadata identity and reports bibliographic total, decided and remaining counts alongside unresolved source count. Local preparation is complete only for a nonempty fully decided worksheet with no unresolved sources. Filled paper decisions do not hide pending attachments, notes or disconnected ancestry. Progress never claims correctness, independent approval, applied reclassification or publication authority. An empty campaign is not complete. The worksheet's own required content identity must match the current report independently of the supplied receipt. Blank identity is invalid; a stale or replaced identity is a snapshot mismatch. Conversion only prepares input for independent verification. Unresolved sources can remain in locally prepared review data, but prevent whole-library completion; refreshing local digests cannot renew an independently issued approval. +Operators must be able to accumulate small steward-reviewed decision sets without hand-merging the complete worksheet. Each patch binds the original library version, classifier revision, snapshot and proposal/retained-content digests, item key and item revision. Regenerating a worksheet after content changes cannot make an older patch valid. Missing content binding requires a new review-bound patch, never automatic backfill. Empty, duplicate, unknown, stale or abstention decisions fail atomically. Identical replay is idempotent; conflicting decisions cannot overwrite review work. Applying a patch does not confer independent approval, full-text review provenance or publication authority. + 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. A complete metadata-review result additionally requires exactly one non-abstention steward label for every top-level bibliographic item and no unresolved source records. Standalone sources, orphan trees and disconnected cycles must be resolved before completion; clearing their reported list cannot bypass inventory validation. A sampled golden set remains valid for quality measurement but cannot prove completion. Neither result proves full-text approval or an applied Zotero reclassification. diff --git a/docs/TRD.md b/docs/TRD.md index 5452a79..f355cd2 100644 --- a/docs/TRD.md +++ b/docs/TRD.md @@ -120,6 +120,8 @@ The owner-only report uses owned JSON values and supports lossless deserializati Offline input admission pins the checked canonical parent plus file name for metadata and opening. Unix opening refuses final-component symlinks with `O_NOFOLLOW` and uses `O_NONBLOCK` so a raced FIFO cannot wait for a writer before device/inode validation rejects the replacement; a pre-open pathname check alone is insufficient. Regular-file reads remain bounded as before. Paired export retains the shared canonical-destination check before capture and serializes both artifacts before writing. Failures may leave private partial files and never trigger pathname cleanup or implicit buffer-flush retry. Finalized metadata remains unverified until the independent whole-set approval boundary succeeds. +`apply_steward_decision_patch` rebuilds the canonical worksheet and validates the complete current worksheet, including rejection of pre-existing reviewed abstention. It validates a nonempty patch against library version, rule revision, snapshot digest and its own required `proposal_digest`; a fresh worksheet cannot renew an older patch's reviewed content. Missing binding fails deserialization; blank or stale binding fails before updates, without backfill. Every update names a unique canonical item key and exact revision with a non-abstention disposition. Updates apply to a clone, so duplicate, unknown, stale or conflicting decisions reject the whole batch without partial state. Identical replay remains idempotent. This is the owner boundary for a later private CLI, not independent approval or full-text/write authority. + 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. The report is local JSON and contains proposals rather than governance decisions. On supported Unix platforms, CLI output is restricted to a new owner-readable/writable (`0600`) direct child of canonical `/tmp` or the operating system temporary directory; exact permissions are restored after umask application, and other platforms fail closed. Relative paths, nested paths, existing paths, and symlinks are rejected, and create-new file semantics prevent overwrite/path-swap writes. Reviewed collection/tag changes can produce a pure local plan whose default mode is dry-run. The plan requires exact report and item preconditions, complete before/after/rollback arrays, externally verified authority, and preserved Zotero tag types; its fields are externally read-only after validation. Zotero 9 execute mode fails closed. Every receipt copies the plan's review, authority, server, Zotero version, library, rule, snapshot and proposal coordinates; dry-run reports every operation as not attempted and makes no Local API call. Execute mode preflights every item before the first write, advances the library precondition only from a directly verified write response, stops on the first adapter or response failure, and re-reads that item through the same boundary as observation only. Failed writes remain indeterminate regardless of observed metadata; no inverse is issued for them. Prior directly verified operations retain their inverse coordinates. The API key remains adapter-owned and absent from serializable structures. diff --git a/docs/adr/0006-zotero-research-intake.md b/docs/adr/0006-zotero-research-intake.md index deacedd..2291bf8 100644 --- a/docs/adr/0006-zotero-research-intake.md +++ b/docs/adr/0006-zotero-research-intake.md @@ -55,6 +55,8 @@ We reject relying only on a pre-open symlink check because a pathname can change The follow-up premortem identified a separate FIFO replacement: `O_NOFOLLOW` does not prevent an open from waiting for a pipe writer before identity validation. No existing owner fix was found. RED `bce7efa` replaces a checked regular unit-test artifact with an actual FIFO and observes a two-second test timeout. `ddf3f62` adds the existing native `O_NONBLOCK` flag at the same open boundary; the open returns without a writer and existing device/inode validation rejects the replacement. This is a filesystem-open safety rule, not a model/application timeout. Regular-file private reads and size limits remain unchanged. Root and later extracted private readers must inherit it; do not transfer this head's test evidence to them. +Incremental steward work is integrated by a snapshot-bound decision patch rather than editing or merging the complete worksheet structure. The patch carries only library/rule/digest coordinates and unique item key/version/disposition updates. The canonical report and current worksheet are revalidated first; applying to a clone makes invalid or conflicting batches atomic failures, while an identical replay is idempotent. We reject direct in-place mutation and last-writer-wins merging because either can silently discard concurrent review work. The patch remains local review input and does not mint governance authority. + The reader fails closed above 50,000 items or 256 MiB of cumulative response bodies, while retaining the 8 MiB per-page bound, finite request timeouts, redirect denial, total-count checks, snapshot-version checks, and duplicate-key detection. Pagination, consistency, resource-budget, and provider-contract behavior are separated from the narrow `ureq` transport so deterministic tests exercise the production reader core rather than excluding the entire reader from coverage. In the context of reading every bibliographic source before classification, facing individually timely pages that can cumulatively hold a run open for days, we decided for a five-minute monotonic admission/completion budget in the existing reader and against rejecting legitimate short pages or adding another transport, to bound accepted work without excluding papers, accepting that an already-started request or classification computation can finish after the limit before its result is rejected. This is an application read limit, not a model timeout, hard process-cancellation deadline, wall-clock/suspend guarantee or atomic snapshot claim. Each page is checked before fetch and after return, and the complete report is checked before return. The stdlib clock has a private deterministic test seam; public APIs, provider timeouts and data/byte ceilings are unchanged. The [deadline doctoring](../doctoring/zotero_metadata_deadline.md) records the original review, RED/GREEN, alternatives and exact verification. This amendment remains Proposed and grants no Zotero mutation authority. @@ -120,6 +122,12 @@ GREEN source `f6735b585022aac1c8ceff86c150d9b64fd77ec2` passed 68 tests across 1 ## Alternatives considered +### Proposed decision-patch content identity amendment — September 7 + +PR31 accumulates local decisions without overwriting conflicts. Normal merge `ef0ce43` preserves the original patch delta and PR30's content-bound worksheet/progress contract. A valid current worksheet alone does not prove that an older patch reviewed the current content: RED `7208d80` compiled with three passing and two failing tests, accepting both an unbound serialized patch and an old patch after changed report context plus fresh worksheet generation. `89bb941` adds a required patch `proposal_digest` and compares it with the existing recomputed identity before any updates. No new hash or authority issuer is added. Missing fields fail loading; blank/stale values cannot equal the expected digest. + +We reject copying the current digest into submitted patches because that would erase the distinction between reviewed and changed content. We reject deriving patch authority from the worksheet because they are separate inputs. Required identity preserves that boundary at the cost of regenerating legacy patches from genuinely reviewed current evidence. Existing clone-before-update behavior remains: a valid first update followed by an unknown update leaves the input worksheet unchanged, identical replay succeeds and conflicting decisions fail. A caller can still construct self-consistent unverified local labels; independent whole-set approval remains mandatory and no metadata patch becomes full-text or Zotero write authority. Later patch CLI/consumer owners must inherit the required field, with protected review and release still pending. + ### Proposed review-progress scope amendment — September 7 PR30 exposes incremental local progress and extracts shared worksheet comparison. Normal merge `7251238` inherits repaired source identity, finalization and private input boundaries while preserving the child's coverage improvements. The extracted comparator omitted the required proposal digest, allowing old worksheet progress after changed content. RED `4099434` compiled with one passing and two failing tests: blank binding was admitted and pending/content identity fields were missing. `bee32f4` adds one shared digest comparison and includes the opaque proposal binding and pending count in existing aggregate output. `b2b0ef4` verifies one filled paper decision plus an unresolved source stays incomplete, preserving bibliographic counts and excluding the private source key. diff --git a/docs/product-technical-gap-baseline.md b/docs/product-technical-gap-baseline.md index 4b8c346..1b4956a 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 PR31 decision-patch content repair + +Original PR31 `61072a70f7ec5a1fbd0b477430aacb8e770aa109` passed 158 tests/33 suites. Normal merge `ef0ce43` retains it and PR30 `ee1ac9925c5287e9f10c2e9581b7cf513b170bd7`; integrated tests passed 204/33. RED `7208d80` compiled with three passing and two failing tests: an old patch applied after changed report context and fresh worksheet generation, and serialized patches without content binding loaded. The valid-first/unknown-later batch already preserved the original worksheet. `89bb941` adds required patch proposal identity and compares it before updates, reusing the existing digest. Missing fields fail loading, stale/blank identities cannot equal the recomputed expected identity, and no automatic backfill is permitted. Independent bounded review found no additional defect; atomic failure, same-label idempotency and conflicting-decision rejection remain. + +Final source `89bb941` passes 207 tests/33 suites including three doctests, strict Clippy, warnings-denied rustdoc, formatting, CI contract and diff checks. Unchanged coverage passes 331/331 reported functions, 2,965/2,965 normalized regions and 544/544 normalized branches. Raw 3,971/4,033 lines, 6,125/6,232 regions and 499/544 branches are not 100%. Logs: `/tmp/conceptweave-pr31-{baseline,integrated,binding-red,verified,clippy,rustdoc,coverage}.log`. PRD/TRD/Proposed ADR0006 trace the separate report/worksheet/patch identities and legacy regeneration requirement. + +Root and later patch consumers must inherit this required binding alongside the prior source, progress and FIFO repairs. Actual decisions and independent approvals remain 0/3,715 plus four unresolved standalone sources. Synthetic fixture labels are not real review evidence. Native Visual Inspection was retried, but the Mac is locked; no fresh screenshot is claimed. Keep Draft. No real Zotero write, hosted GREEN, protected merge or release occurred. + ### September 7 PR30 content-bound progress repair Original PR30 `a11e889d1680ab4d91f3565e3debf7ed0f10ba23` passed 156 tests/32 suites. Normal merge `7251238` retains it and PR29 `e21f14fbb4954762ffc97521af9a6cdd9982c630`; integrated tests passed 200/32. Child private-helper coverage improvements remain, combined with parent FIFO/source/approval/output safety. RED `4099434` compiled with one passing and two failing progress tests: blank proposal binding was accepted and pending/content identity fields were absent. `bee32f4` compares the existing digest in the shared worksheet validator and adds opaque proposal identity plus pending count to aggregate progress. `b2b0ef4` proves filled bibliographic slots do not hide an unresolved standalone source. Independent bounded review found no collateral finalization error-precedence or privacy regression.