feat(research): add reviewed Zotero dry-run write plan - #13
Conversation
…oresearch/zotero-reviewed-write-plan # Conflicts: # docs/UBIQUITOUS_LANGUAGE.md
|
@codex review |
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
@codex review |
…view-manifest' into autoresearch/zotero-reviewed-write-plan
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: da89b31fd2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| Steward->>Intake: verified collection/tag changes | ||
| Intake->>Report: dry-run write plan with exact rollback state |
There was a problem hiding this comment.
The new steward-to-intake collection/tag review flow introduces a governance relationship that is absent from docs/CONTEXT_MAP.md: its Research Intake relationship still describes only duplicate-set verification and canonical-key operations. Update that relationship to include classification-write reviews and write-plan authority so the documented bounded-context contract matches this sequence.
AGENTS.md reference: AGENTS.md:L11-L11
Useful? React with 👍 / 👎.
| if change.item_version != item.item_version | ||
| || before_collections != actual_collections | ||
| || before_tags != actual_tags |
There was a problem hiding this comment.
When a report comes from the public classify_snapshot API with duplicate raw keys—for example, bibliographic A at version 7 and a child also keyed A at version 8—only classified_items is deduplicated, so this comparison accepts version 7 even though snapshot_items makes the key ambiguous. A mutated ClassifiedItem can likewise diverge from the digest-bound coordinates. Build a unique key/version map from report.snapshot_items and require every targeted classified item to match it before emitting a snapshot-bound operation.
AGENTS.md reference: AGENTS.md:L18-L18
Useful? React with 👍 / 👎.
| if collection_keys.iter().any(|key| key.trim().is_empty()) | ||
| || tags.iter().any(|tag| tag.tag.trim().is_empty()) | ||
| || collection_keys.iter().collect::<BTreeSet<_>>().len() != collection_keys.len() |
There was a problem hiding this comment.
When a reviewed after-state contains an unsupported provider value such as tag_type: Some(2), this validation accepts it and serializes it into an execute-capable plan because it checks only whether the tag text is blank or duplicated. Constrain the marker to Zotero's supported manual/automatic representations so a later adapter cannot receive a plan whose requested or rollback tag state the provider cannot reproduce.
AGENTS.md reference: AGENTS.md:L14-L14
Useful? React with 👍 / 👎.
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 7d2b04d51d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| .iter() | ||
| .map(|tag| tag.tag.as_str()) | ||
| .collect::<BTreeSet<_>>() | ||
| .len() | ||
| != tags.len() |
There was a problem hiding this comment.
When a manual Zotero tag is represented with an omitted type in one state and type: 0 in another, Zotero treats both representations as manual, but this normalization retains None and Some(0) as distinct values. Consequently an otherwise exact reviewed before-state can be rejected as StaleItem, while a change containing only that representation difference can evade the NoChange check and produce a meaningless write operation; canonicalize the manual marker before comparisons.
AGENTS.md reference: AGENTS.md:L14-L14
Useful? React with 👍 / 👎.
| if mode == WriteMode::Execute | ||
| && report | ||
| .zotero_version |
There was a problem hiding this comment.
When an otherwise valid reviewed report originated from Zotero 9, changing only the public report.zotero_version field to "10" makes this check emit an Execute plan because that version is neither covered by the raw-item digest nor carried by ReviewedClassificationWriteSet. The review, server, library, rule, digest, and item checks still pass, so the explicit Zotero 9 fail-closed invariant can be bypassed and the resulting plan does not retain the version used to establish write capability; bind the observed Zotero version to the reviewed provenance instead of trusting this mutable field.
AGENTS.md reference: AGENTS.md:L14-L14
Useful? React with 👍 / 👎.
…view-manifest' into autoresearch/zotero-reviewed-write-plan # Conflicts: # docs/product-technical-gap-baseline.md
|
@codex review |
…view-manifest' into autoresearch/zotero-reviewed-write-plan
|
Codex Review: Didn't find any major issues. Hooray! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
|
@codex review |
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Non-force two-parent restack onto the current duplicate-review parent. Preserve the write-plan delta while inheriting the repaired Foundation CI contract through the dependency chain. Signed-off-by: Seongho Bae <me@seonghobae.me>
|
Current-stack correction (2026-09-05): exact base |
# Conflicts: # docs/PRD.md
1 participant
Verified local approval-order repair — 2026-09-06 checkpoint
Exact head:
autoresearch/zotero-reviewed-write-plan@8a684882005085d8b3cb47812e185975084e0475. Exact base:autoresearch/zotero-duplicate-review-manifest@ee2c494a3136a4bfa520c29f1c938b621e1ecb9c.Original planner owner #13 preserves regression
505e111c993d8269e5b7b9e17a25a5ce20f8606eand repair8a684882005085d8b3cb47812e185975084e0475. Every existing local request/mode/item/metadata check finishes before the external approval verifier. Invalid requests invoke it zero times; valid complete requests invoke it exactly once. Local validation errors intentionally precede approval denial. Deterministic operations and complete before/after/rollback metadata are unchanged.This exact head passed locked Rust 1.98.0 workspace tests (72 tests / 18 unfiltered suites, doctests included), strict all-target Clippy, formatting, warnings-denied rustdoc, CI contract and diff checks before normal push. This original owner also passed the unchanged coverage gate: 143/143 functions, 1357/1357 source-normalized regions and 244/244 normalized branches; raw LLVM is 1508/1530 lines, 2126/2161 regions and 206/244 branches, not 100%.
Keep Draft behind the existing prerequisite stack. This is local verification, not hosted current-head GREEN, independent approval, protected merge or release. No later full-text feature was reverse-merged into an earlier owner. Full-text-aware write admission, authentic decisions and independent authority remain separate gaps; no real Zotero/model request, label, approval or write was performed for this repair.
Earlier coordinates and status claims below are historical.
Prior PR description, retained without discarding evidence
Current source-integrity note — 2026-09-05
autoresearch/zotero-reviewed-write-plan@e35f42e2a552ba8da26250bf084462a36d1c986d.autoresearch/zotero-duplicate-review-manifest@3bed88707b347a6db52ea9722f5a1c8f00933813.e7d4e59f1b55b5954c5f8436527bc96e7ef2fb13through ordinary merge ancestry. The source-snapshot digest binds complete captured raw provider JSON and the actual typed classifier inputs; source evidence and derived proposals retain separate identities.GoldenSetApproval.proposal_digestis required and binds the complete proposal records used for evaluation. The current proposal digest is checked before the caller-owned governance verifier. Do not backfill old receipts: regenerate evidence and obtain a new approval bound to the reviewed evidence.Earlier heads, runtime snapshots, campaign counts, and verification statements below are historical notes, not current acceptance evidence.
Historical PR notes — original text retained
Outcome
Adds a governance-bound classification write plan whose default is dry-run. Every planned operation is bound to exact review/authority/server/library/rule/snapshot/item coordinates and complete collection/tag before-state, preserves deterministic before/after/rollback values, and performs no HTTP mutation in this PR.
Current exact stack — 2026-09-05
autoresearch/zotero-duplicate-review-manifest@5ce1a186e0235843abc6e39ebedbb9a4d5501320;821b04c640e798d851a21d893f9aa3d7d157470c;821b04c...; predecessor/local evidence is not GREEN.Review -> repair state
The lineage keeps the reviewed classification-write invariants at the source boundary:
docs/CONTEXT_MAP.mdrecordsResearch Intake -> Governance & Publicationas the ACL through which Governance verifies complete duplicate and classification-write review sets and returns only opaque authority receipts before Intake emits canonical-key operations or write plans;reviewed.snapshot_itemsmust equalreport.snapshot_items, snapshot keys must be nonblank/unique, and every targeted classified item revision must equal the unique digest-bound snapshot revision;typeomitted andtype: 0canonicalize to the same domain value before stale/no-change comparison;reviewed.zotero_version == report.zotero_version, plus server/library/rule/snapshot coordinates;Previously returned review findings with source repairs remain acceptance-gated until an unchanged exact head executes required Product/coverage/rustdoc/security evidence.
Current write-plan boundary
ClassificationWritePlanis local-only and snapshot-bound. Unknown/duplicate/blank targets, stale or ambiguous item coordinates, malformed collection/tag metadata, no-op changes, unsupported Zotero tag types,NeedsStewardReview, unsupported Zotero major versions and missing execute server identity fail closed. Operations retain complete normalized before/after/rollback collection and typed-tag state.No API key is stored, no network call is made, source records/attachments are not deleted, and ADR 0007 remains
Proposed. The later execution/transport/authorization lanes do not back-prove this plan.Dependency boundary
Keep Draft behind #12 -> #11 -> #10 -> #9 -> Foundation. #15 and later descendants must retain the current #13 ancestry. Do not perform Zotero write-back, promote candidate classification to semantic authority, resolve valid review findings before required verification, or transfer predecessor/cancelled evidence.
Acceptance boundary
Issue #8 AC5 is materially advanced. AC6 remains incomplete until authenticated Zotero 10+ transport plus user-authorized live write, partial-failure receipt, rollback receipt and source-preservation evidence are verified through their own dependent lanes. No self-approval, routine bypass, force-push, destructive rebase, mutable supplier dependency, or premature authority promotion.
Refs #8.