andodeki
ad3fe19b90
Some checks failed
repo hygiene / hygiene (push) Has been cancelled
Ten PDF feature commits landed without an ADR, covering three whole phases.
Every feature gets one; these are the four that were owed. Written against
the code as it stands and re-verified by running it, not transcribed from
the commit messages.
0020 CCITT, JBIG2 and JPEG 2000 — the codecs ADR 0015 refused by name
0021 Phase 4 completion — stamping, reconciliation, CFF, cmap, and the
audit that corrected a false "complete" in ADR 0019
0022 Editing — content_edit, page_ops, flatten, catalog_edit
0023 Redaction and compaction, and the three reader defects they found
Verified rather than assumed, on the tree at f75c1cc:
ccitt 30, jbig2 29, jpx 44 unit tests; jpx_roundtrip's 7 compare against
OpenJPEG-generated pixels, exactly, because a JPEG 2000 bug produces a
plausible image rather than an error.
Redaction re-checked with a test written from outside the module. The
middle line is the one worth keeping:
after redact, secret present = true <- earlier revision
after compact, secret present = false
public text retained = true
Redaction alone leaves the secret in the bytes. That is why the two
shipped together, and why the report carries
earlier_revisions_retain_content.
Determinism re-checked by generating the same document from four separate
processes: byte-identical. The HashMap key-order defect really is fixed.
This ADR contains a correction to its own first draft. 0022 initially
listed text-run rewriting and annotation sub-types as deferred, on the
strength of a grep for "rewrite" finding nothing. Both are implemented —
the rewriter is ContentEditor::set_text, which preserves the operator kind
so a ' keeps its line advance and a TJ keeps its kerning; and "callout" and
"comment" are dart-pdf's names for a FreeText with a /CL and a Text
annotation, not distinct /Subtype values. Checking the plan's claim against
the code was right; concluding from a failed grep was not.
One criterion across the four is left unticked, and it is real: the ui.rs
interaction tests, blocked on the Makepad headless backend since Phase 1.
0021 also records a process note. ADR 0019's merge criteria were derived
from what had been built, so all of them ticked while three unbuilt items
stayed invisible. Criteria should come from the plan text first, then be
marked done or explicitly deferred — a deferral stated is fine, a deferral
unstated is a false claim.
pdf: 1187 passed. No code changed.
152 lines
6.9 KiB
Markdown
152 lines
6.9 KiB
Markdown
# ADR 0022: editing — content streams, page operations, flatten, and catalogue edits
|
||
|
||
- **Status:** Accepted
|
||
- **Date:** 2026-08-17
|
||
- **Review item:** `NIGIG_PDF_FEATURE_PARITY_PLAN.md` §1 Phase 5, "Editing
|
||
(pdf-document)"
|
||
- **Supersedes:** nothing
|
||
- **Related:** ADR 0019 (creation — this is its inverse), ADR 0023
|
||
(redaction and compaction, the remaining Phase 5 items)
|
||
- **Written retrospectively.** The work landed in `b8bd718`, `e0aab74`,
|
||
`e0d8627` and `b870dc4` without an ADR.
|
||
|
||
## Context
|
||
|
||
Phases 1–3 read PDFs; Phase 4 wrote them from scratch. Neither could change
|
||
an existing document without rewriting it wholesale, which loses everything
|
||
the original file contained that the model does not represent — and that is
|
||
always a lot.
|
||
|
||
Editing needs a different property from creation: whatever the editor does
|
||
not understand must survive untouched.
|
||
|
||
## Decision
|
||
|
||
Four modules, following the crate boundary: content-stream work sits in
|
||
`pdf-graphics` because it needs the operator model; document-structure work
|
||
sits in `pdf-document`.
|
||
|
||
### `content_edit.rs` — the serialiser (1,257 lines, 45 tests)
|
||
|
||
The foundation, and the piece that had to exist first. `parse_content_stream`
|
||
had always been one-way: operators in, nothing out. `write_op` and
|
||
`write_ops` complete the round trip, and `ContentEditor` provides
|
||
range-addressed insert, delete and replace over an operator list.
|
||
|
||
`write_real` is separate and exported for the same reason `format_real`
|
||
exists in the writer (ADR 0019): PDF's number grammar has no exponent form,
|
||
so `format!("{}", 1e21)` produces something no parser accepts. A content
|
||
stream is a second place that mistake can be made, so it has its own tested
|
||
function rather than a second ad-hoc one.
|
||
|
||
Edits are addressed by `OpRange`, not by byte offset. Byte offsets shift
|
||
under every preceding edit; operator indices are stable within one editing
|
||
session, which is what a caller can actually reason about.
|
||
|
||
### `page_ops.rs` — insert, reorder, duplicate, delete, import (852 lines, 16 tests)
|
||
|
||
The design decision worth recording is `PageIndexMap`. Every operation
|
||
returns one, mapping original index to new index, with `was_deleted` for
|
||
pages that are gone. This exists because *everything else in a document
|
||
refers to pages*: outlines, named destinations, link annotations, structure
|
||
trees. Reordering pages without a mapping silently breaks all of them, and
|
||
breaks them in the way this project keeps finding — the link still resolves,
|
||
to the wrong page.
|
||
|
||
Import merges pages from another document, carrying the resources those
|
||
pages depend on.
|
||
|
||
### `flatten.rs` — annotations and forms into page content (837 lines, 11 tests)
|
||
|
||
Flattening draws each annotation's appearance stream into the page and
|
||
removes the annotation, producing a file that looks identical in a viewer
|
||
that does not render annotations at all.
|
||
|
||
The subtle part is `appearance_matrix`: a form XObject's `/BBox` is in its
|
||
own space with its own `/Matrix`, and it must be mapped onto the
|
||
annotation's `/Rect`. Getting that wrong scales or offsets every flattened
|
||
annotation — the failure looks like a layout bug, not a maths bug, so it
|
||
gets its own exported and separately tested function.
|
||
|
||
`remove_acroform_if_empty` cleans up `/AcroForm` once its fields are gone,
|
||
because a form dictionary with no fields makes a viewer offer form
|
||
behaviour for a document that no longer has any.
|
||
|
||
### `catalog_edit.rs` — outlines, page labels, struct tree (533 lines, 7 tests)
|
||
|
||
The write side of what ADR 0019 created and `catalog.rs` reads.
|
||
`remove_structure_tree` exists for flatten and redaction: a structure tree
|
||
that still describes removed content is worse than none, because assistive
|
||
technology reads it and announces text that is no longer on the page.
|
||
|
||
## Verification
|
||
|
||
Re-run against the tree at `f75c1cc`:
|
||
|
||
```
|
||
content_edit 45 #[test] (pdf-graphics lib: 289 passed)
|
||
page_ops 16 passed
|
||
flatten 11 passed
|
||
catalog_edit 7 passed
|
||
TEST_TARGET=pdf 1187 passed
|
||
TEST_TARGET=pdf-ui 1232 passed
|
||
```
|
||
|
||
## Merge criteria
|
||
|
||
Taken from the Phase 5 plan text, then marked — not derived from what was
|
||
built (see ADR 0021's process note):
|
||
|
||
- [x] Content-stream editing: insert/delete/replace operator runs
|
||
- [x] Page content mutation with xref/revision-safe save
|
||
- [x] Page management: insert, reorder, duplicate, delete
|
||
- [x] Page import/merge from another document
|
||
- [x] Flatten annotations and forms into page content
|
||
- [x] Outlines editing
|
||
- [x] Page labels editing
|
||
- [x] Struct-tree editing
|
||
- [x] Object compaction — **ADR 0023**
|
||
- [x] Redaction — **ADR 0023**
|
||
- [x] Annotation editing and appearance regeneration — pre-existing
|
||
(`annotation_edit.rs`, `appearance.rs`, ADR 0004)
|
||
- [x] Text-run rewriting (`content_run_rewriter.dart`) —
|
||
`ContentEditor::set_text` and `text_op_indices`. Preserves the
|
||
operator *kind*, so a `'` keeps its implicit line advance and a `TJ`
|
||
keeps its kerning numbers while its strings are replaced
|
||
- [x] Annotation sub-type coverage — every subtype PDF defines is parsed
|
||
(`Text`, `Link`, `FreeText`, `Square`, `Circle`, `Line`, `Polygon`,
|
||
`PolyLine`, `Highlight`, `Underline`, `StrikeOut`, `Squiggly`, `Ink`,
|
||
`Stamp`, `Popup`, `Widget`, `Redact`). "Callout" and "comment/note"
|
||
are dart-pdf's names for a `FreeText` with a `/CL` callout line and a
|
||
`Text` annotation respectively, not distinct `/Subtype` values
|
||
- [ ] **`ui.rs` annotation interaction tests** — blocked on the Makepad
|
||
headless backend, as they have been since Phase 1. The only item in
|
||
this phase that is genuinely outstanding.
|
||
|
||
## Consequences
|
||
|
||
**Positive.** A document can be modified rather than regenerated, so
|
||
everything the model does not represent survives an edit. `PageIndexMap`
|
||
makes the cross-reference problem explicit instead of leaving each caller to
|
||
discover it.
|
||
|
||
**Negative.** Four modules, 3,479 lines, and the invariant that edits must
|
||
preserve unknown structure is not expressible as a type — it is a property
|
||
each function has to maintain by hand, and a reviewer has to check.
|
||
|
||
**Risk.** `OpRange` indices are stable only within one editing session. A
|
||
caller that holds a range across a mutation is addressing something else,
|
||
and nothing in the type system prevents it. This is documented; it would be
|
||
better enforced by a generation counter on the editor.
|
||
|
||
**A correction to this ADR's own first draft.** It initially listed
|
||
text-run rewriting and annotation sub-types as deferred, on the strength of
|
||
a grep for `rewrite` finding nothing. Both are implemented — the rewriter is
|
||
`ContentEditor::set_text`, and the "missing" annotation types are dart-pdf's
|
||
names for shapes PDF spells differently. Checking the plan's claim against
|
||
the code was right; concluding from a failed grep was not. The lesson is
|
||
ADR 0021's in reverse: a name that does not appear is not a feature that
|
||
does not exist.
|
||
|
||
The only genuinely outstanding Phase 5 item is the `ui.rs` interaction half,
|
||
blocked since Phase 1 on the Makepad headless backend.
|