nigig-org/REVIEWS/adr/0004-pdf-annotation-editing.md

ADR 0004: PDF annotation editing — typed edits on document-owned state

• Status: Accepted
• Date: 2026-07-28
• Review item: Phase 8, DART_PDF_VS_MAKEPAD_PDF_GAP_ANALYSIS.md
• Supersedes: nothing
• Related: ADR 0003 (incremental save), review phases 3 and 4

Context

The gap analysis lists annotation editing twice:

PdfAnnotationEditing — move, resize, restyle, rotate annotations — MISSING

Its prerequisites are "document model, appearance generation, incremental save". All three now exist: the document model landed in Phase 3, appearance generation with it, and incremental save in ADR 0003.

Annotations today are strictly read-only. PdfAnnotation exposes public fields and from_dict, and there is not a single mutator or &mut self method in the module. The viewer can find a link and report a click on it (Phase 4), but nothing can move a highlight, restyle a square, or delete a stamp.

This is chosen ahead of the other unblocked feature, AcroForm full support, for one reason: the review lists that feature's prerequisites as "document model, appearance regeneration, JavaScript actions". A scripting engine is a large new dependency and a security surface in its own right — running document-supplied code is exactly the class of decision that deserves its own ADR and threat review, not a side effect of finishing a form feature. Annotation editing needs nothing that does not already exist.

Decision

Mirror the shape that already works for forms. DocumentFormEditor takes typed edit commands, validates them, mutates document-owned state and returns real errors; annotation editing gets the same contract rather than a second, differently-shaped API.

The editing model

AnnotationEditor applies typed AnnotationEdit values to a document-owned PageAnnotations collection:

• Move { to } and Resize { rect } — geometry.
• SetColor, SetBorderWidth, SetOpacity — appearance.
• SetContents — the note text.
• SetFlags — hidden, print, read-only.
• Delete — removal.

Every edit is validated before anything changes, so a rejected edit leaves the annotation untouched. This is the property the form editor already has and the one that makes an editor safe to wire to a UI.

Identity

Annotations are keyed by ObjRef, as fields are. A name or an index into a /Annots array is not stable across a save, and Phase 3 already established that page-0-style fallbacks are how the wrong object gets edited.

PdfAnnotation currently carries no obj_ref. It gains one, populated by PdfDocument::page_annotations, which already resolves each entry through the xref.

Scope

In scope: move, resize, restyle, set contents, set flags, delete; saving those edits through the ADR 0003 incremental writer; regenerating appearance streams for the annotation types whose look this crate can actually draw.

Explicitly out of scope, recorded rather than implied:

• Creating new annotations. Editing existing ones is the review's wording, and creation needs a page /Annots rewrite plus placement policy that belongs with the UI work.
• Appearance generation for types this crate cannot draw: Stamp, 3D, Screen, Sound, Movie, Redact. An edit to one of these saves its dictionary and keeps the document's existing /AP rather than replacing it with something wrong.
• Redaction application. Marking a redact annotation is an edit; burning it into the page content is a destructive operation needing its own design.
• Rich text (/RC) and measurement annotations.

Non-negotiable rules

1. A rejected edit changes nothing. Validated before mutation, as with forms.
2. Deleting an annotation removes it from the page's /Annots, not just from an in-memory list. A save that leaves a dangling reference produces a file other readers reject.
3. An annotation whose appearance cannot be generated keeps its existing /AP. Writing a blank appearance would erase a stamp's artwork.
4. Read-only annotations refuse edits unless the caller explicitly overrides, mirroring the form editor's ReadOnly error.
5. Geometry is validated. A rectangle with zero or negative extent is refused, not normalised silently: it usually means a UI bug upstream.

Merge criteria

• Move, resize, restyle, contents, flags and delete all apply and are observable on the in-memory model.
• Each is refused with a typed error where invalid, leaving state untouched.
• Edits save through the incremental writer and survive a reparse.
• A deleted annotation is absent from the reparsed page's annotations.
• An annotation type with no appearance generator keeps its original /AP and is reported as skipped.
• Corpus round trip on annotations/links.pdf and hidden.pdf.
• Malformed corpus fixtures never panic an annotation edit or save.
• TEST_TARGET=pdf ./tools/test-rust-clean.sh passes, rustfmt and clippy -D warnings clean.

All criteria met, but not all of them were met when this ADR was written. The audit that ticked these boxes found two genuine gaps rather than paperwork:

1. "reported as skipped" was never implemented for annotations. Rule 3's preservation worked — EditableAnnotation::to_dict clones the source dictionary, so a stamp's /AP was always kept — but SaveReport had no field for it. Only the form path reported skipped appearances. A caller who moved a stamp was never told its artwork still showed the old position. Closed by SaveReport::annotation_appearances_skipped and appearance_is_generated, which enumerates the out-of-scope types explicitly so a newly added AnnotationType fails to compile until it is classified.
2. SetContents and SetFlags had no round-trip test. Both were implemented and unit-tested against the in-memory model, but neither was ever reparsed from written bytes — the assertion this ADR calls central.

New fixture annotations/stamp.pdf carries real /AP artwork for a Stamp (undrawable, must be preserved and reported) alongside a Square (drawable, must not be reported) so the reporting cannot pass by reporting everything.

Consequences

Positive. The viewer becomes an annotator. The save path generalises from "form edits" to "document edits", which is the shape the remaining Phase 8 features need. Phase 4's interaction layer gains something to call beyond form fields.

Negative. Two editors now exist, for forms and annotations, over the same document. They are deliberately kept separate because their validation rules differ, but a future document-wide edit journal would subsume both.

Risk. The likeliest defect is a deleted annotation left referenced by its page, producing a file this viewer reads and Acrobat rejects. The mitigation is a round-trip test asserting the reference is gone from the reparsed page, not merely from the model.