Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
64 changes: 64 additions & 0 deletions openspec/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
# SAPP fork — OpenSpec

This folder contains the implementation contracts for the
text-replacement feature being staged on the `work/text-replacement`
integration branch of `ConductionNL/sapp`. Each change corresponds to
one PR that we eventually intend to contribute back upstream to
`dealfonso/sapp` once the feature stabilises in our primary consumer
(OpenRegister's PDF anonymisation pipeline).

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Concern — README slash-command syntax doesn't match the loaded skills

The openspec/README.md Workflow section writes commands as /opsx:ff, /opsx:continue, /opsx:apply, /opsx:verify, /opsx:archive — the colon-separated form. The currently-loaded skills use the hyphen form: opsx-ff, opsx-continue, opsx-apply, opsx-verify, opsx-archive.

Impact: A developer reading the README and typing /opsx:ff will hit a 'skill not found' error.

Suggested fix: Rewrite the five workflow bullets as /opsx-ff, /opsx-continue, /opsx-apply, /opsx-verify, /opsx-archive.


## Why an OpenSpec scaffold inside a PHP library?

SAPP is not a Nextcloud app, but the text-replacement work is being
done under time pressure for a Nextcloud-app consumer (OpenRegister)
that lives in the wider Conduction OpenSpec workflow. Putting the
specs next to the code that implements them keeps the contract close
to the source, makes upstream-PR submission easier later (each spec
becomes the PR's design doc), and gives us a place to record the
implementation notes worth surfacing in the upstream review.

The `docs/upstream-prs/01-asciihex-decode/` ... `08-text-replacement-api/`
folders are the actual artefacts we'll paste into the upstream PRs.
These directories are created lazily by the individual implementation
PRs; this scaffold change ships only the `openspec/` tree. The
`openspec/changes/` folder is where the work is planned and tracked
**before** those upstream-PR-draft folders get the polished final
write-up.

## Folder structure

| File / Folder | Purpose |
|---|---|
| `config.yaml` | OpenSpec project config — rules tuned for a PHP library, not a Nextcloud app |
| `architecture/` | ADRs specific to the fork (e.g. fork-and-give-back ordering) |
| `specs/` | Accepted capability specs — created when a `change/` is archived |
| `changes/` | One folder per planned upstream PR — proposal, design, spec, tasks |

## Mapping to the upstream-PR series

Rows are listed in dependency order — `feat-filter-chain-dispatch` is
the foundation that PRs `#01`–`#04` attach to. The "Upstream-PR draft"
column references the `docs/upstream-prs/NN-<slug>/` folder index,
which intentionally differs from the dependency-order rows. To
disambiguate "upstream-PR draft" indices from ConductionNL/sapp PR
numbers throughout the rest of this scaffold, references are written
as `upstream-PR #NN` (or by change name).

| OpenSpec change (dependency order) | Upstream-PR draft | PDF 1.7 ref |
|---|---|---|
| `feat-filter-chain-dispatch` (foundation) | `docs/upstream-prs/05-filter-chaining/` | §7.4 (Filters, array form) |
| `feat-asciihex-decode` | `docs/upstream-prs/01-asciihex-decode/` | §7.4.2 |
| `feat-runlength-decode` | `docs/upstream-prs/02-runlength-decode/` | §7.4.5 |
| `feat-ascii85-decode` | `docs/upstream-prs/03-ascii85-decode/` | §7.4.3 |
| `feat-lzw-decode` | `docs/upstream-prs/04-lzw-decode/` | §7.4.4 |
| `feat-tounicode-cmap` | `docs/upstream-prs/06-tounicode-cmap/` | §9.10 (ToUnicode CMaps) |

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Minor — Mapping table presents the 8 changes out of dependency order

The table lists changes in dependency order (foundation first), but the right-most column (upstream-PR index) is sorted as 05, 01, 03, 04, 02, 06, 07, 08 — jumbled.

Suggested fix: Add a one-line note above the table explaining rows are in dependency order, or add an explicit 'Depends on' column.

| `feat-tj-flattening` | `docs/upstream-prs/07-tj-flattening/` | §9.4 (text-showing operators) |
| `feat-text-replacement-api` | `docs/upstream-prs/08-text-replacement-api/` | n/a (new public API) |

## Workflow

- `/opsx-ff` — Create + scaffold a new change end-to-end
- `/opsx-continue` — Resume work on an in-progress change
- `/opsx-apply` — Drive the implementation from `tasks.md`
- `/opsx-verify` — Verify implementation matches the spec
- `/opsx-archive` — Move a completed change into `specs/`
Empty file added openspec/architecture/.gitkeep
Empty file.
Empty file added openspec/changes/.gitkeep
Empty file.
2 changes: 2 additions & 0 deletions openspec/changes/feat-ascii85-decode/.openspec.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
schema: spec-driven

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Minor — Bytewise-identical 2-line .openspec.yaml files

Every change-folder ships a 2-line .openspec.yaml with the same schema and created. No per-change identifying field.

Suggested fix: Either drop the file (folder name is the identity) or extend the schema to include name:, status:, capabilities:.

created: 2026-05-26
60 changes: 60 additions & 0 deletions openspec/changes/feat-ascii85-decode/design.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
## Context

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Concern — Migration Plan section asymmetry

The design.md files break into two camps: WITH ## Migration Plan (feat-asciihex-decode, feat-filter-chain-dispatch, feat-text-replacement-api, feat-tounicode-cmap) and WITHOUT (feat-ascii85-decode, feat-lzw-decode, feat-runlength-decode, feat-tj-flattening). All eight changes are 'strictly additive', so if Migration Plan is needed for one it is needed for all.

Suggested fix: Add ## Migration Plan to the four missing files (the content can be 'No migration needed — strictly additive').


ASCII85 base-85 encoding maps 4 binary bytes (interpreted as a 32-bit unsigned big-endian integer `n`) to 5 ASCII characters via `n = c0 * 85^4 + c1 * 85^3 + c2 * 85^2 + c3 * 85 + c4` where each `ci` is in `[0, 84]` and emitted as `ci + 33` (codepoint range `!..u`).

Special cases:

- Single character `z` decodes to 4 zero bytes (compact form for runs of zeros — common in image data).
- EOD marker `~>` terminates decoding.
- Whitespace anywhere is ignored.
- Trailing partial groups: a group of `k` ASCII chars (where `2 ≤ k ≤ 4`) decodes to `k - 1` binary bytes, padded with `u` (codepoint 117 = value 84) to a full 5-char group before computing.

## Goals / Non-Goals

**Goals:**

- Lossless round-trip on arbitrary binary input.
- Honour the `z` shortcut on both encode and decode.
- Tolerate whitespace anywhere in the encoded stream.

**Non-Goals:**

- The alternative "btoa version 4.2" syntax with `<~` start marker — PDF 1.7 §7.4.3 doesn't use it and Adobe Acrobat doesn't emit it.
- ASCII85 in other contexts (PostScript level-2 has a slightly different EOD convention).

## Decisions

### D1 — Mirror upstream's filter helper shape

`protected static function ASCII85Decode($_stream, $params)` and `protected static function ASCII85Encode($_stream, $params)` as static helpers on `PDFObject`.

### D2 — Use PHP's native arithmetic, not `gmp` or `bcmath`

The 32-bit big-endian integer never exceeds `2^32 - 1`. PHP's `int` type is at least 32-bit (64-bit on 64-bit platforms, which sapp's `composer.json` requires via the PHP 7.4 floor). No external math extension needed.

### D3 — Encode emits `z` for runs of 4 zero bytes (greedy, aligned)

The encoder emits `z` only when 4 consecutive zero bytes are aligned on a group boundary. Unaligned zero runs go through the standard 5-char encoding. This matches Adobe's de-facto encoding behaviour and keeps the implementation simple.

### D4 — Decode rejects illegal characters via `p_error` + raw input return

Characters outside `!..u` plus `z` plus whitespace plus the EOD bytes `~>` cause `p_error()` and a return of the raw input. Same convention as the other filters.

### D5 — Decode tolerates missing EOD (Adobe-compatible)

If the stream ends without a `~>` marker, the decoder finishes the last partial group (treating trailing `u`s as padding) and returns the result. Adobe Reader does this. Spec is mildly ambiguous on whether EOD is mandatory; the lenient interpretation is reader-compatible.

## Risks / Trade-offs

- **Risk**: A 5-char group might overflow `2^32 - 1` if all 5 chars are at their maximum value (`uuuuu` = `85^4 * 84 + ... + 84 = 4294967295`). → **Mitigation**: that's exactly `2^32 - 1`, the maximum representable value; PHP's int handles it. Groups beyond this value (`uuuu\x76` would compute to > `2^32`) are spec-illegal — the decoder fails them via the `p_error` path.

- **Trade-off**: Greedy z-encoding leaves a few bytes on the floor when zero runs straddle group boundaries. → **Mitigation**: documented; in practice this matters only for image data which we don't anonymise.

## Open Questions

- **OQ1**: Some readers tolerate the `<~` start marker (btoa convention). PDF 1.7 §7.4.3 doesn't mention it. Reject or tolerate? **Provisional**: tolerate on decode (strip the leading `<~` if present); never emit on encode. Maximum reader-compatibility on input; spec-faithful on output.

## Migration Plan

Strictly additive change. No migration required for consumers — string-form callers (and unaware-of-this-change callers) observe byte-for-byte identical behaviour after merge. Rollback path is a clean revert of the commit.

29 changes: 29 additions & 0 deletions openspec/changes/feat-ascii85-decode/proposal.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
## Why

`ASCII85Decode` (PDF 1.7 §7.4.3) encodes 4 binary bytes per 5 printable ASCII characters in the range `!..u`, with a single-character shortcut `z` for four zero bytes. It's used as an outer envelope in PDFs that need to pass through 7-bit-clean transports while still being more compact than ASCIIHex. Real-world appearances include certain Adobe Distiller outputs and PDFs produced by older Mac toolchains. Upstream sapp doesn't implement it; without it, any chain-encoded content stream that uses an ASCII85 outer wrapper falls through to the no-op path and the text-replacement pipeline produces corrupted output on those PDFs.

## What Changes

- Add `ASCII85Decode` encode + decode primitives in `PDFObject`.
- Wire `/ASCII85Decode` into the chain dispatcher introduced by `feat-filter-chain-dispatch`.
- Spec-faithful per §7.4.3:
- Decode: 5-char groups in `!..u` (codepoints 33..117) decode to 4 bytes via base-85. The shortcut `z` decodes to four zero bytes. EOD marker is `~>`. Trailing groups of fewer than 5 chars decode to fewer than 4 bytes (1 → impossible; 2 → 1 byte; 3 → 2 bytes; 4 → 3 bytes), padded with `u` (codepoint 117) for the missing chars.
- Whitespace anywhere is ignored.
- No `/DecodeParms` (Table 5).

## Capabilities

### New Capabilities

- `ascii85-decode-filter`: PDF 1.7 §7.4.3 ASCII85Decode encode + decode plugged into the filter chain dispatcher.

### Modified Capabilities

- `filter-chain-dispatch`: extend the `case` table with `/ASCII85Decode`.

## Impact

- **Touched files**: `src/PDFObject.php` (+ASCII85 helpers, +dispatch arms; ~80 LOC).
- **Public API**: none.
- **Depends on**: `feat-filter-chain-dispatch`.
- **Upstream-PR draft**: `docs/upstream-prs/03-ascii85-decode/`.
Original file line number Diff line number Diff line change
@@ -0,0 +1,127 @@
**Status**: planned
**Scope**: change `feat-ascii85-decode` (delta spec)
**OpenSpec changes**:
- `feat-ascii85-decode` (in-progress)

## Purpose

Capability contract for `ascii85-decode-filter` — the normative SHALL/MUST
behaviour the change must deliver. Scenarios below are the
acceptance criteria; tasks under the change's `tasks.md` reference
these requirements by REQ-NNN id.

## Non-Functional Requirements

- PHP >= 7.4 compatibility per upstream sapp's composer constraint.
- Zero new composer dependencies.
- snake_case method names on new utility helpers; PascalCase on
filter names (matches the existing `FlateDecode` convention).
- All round-trip scenarios MUST be lossless byte-for-byte unless
the spec explicitly documents a deviation.

## Acceptance Criteria

- Every requirement below MUST be exercised by at least one scenario
in the change's verification gate under `examples/`.
- The change's `tasks.md` MUST cite each REQ-NNN it implements.
- Existing verification gates (PoC and prior changes) MUST remain
green after the change lands.

## Notes

This is a delta spec — the canonical spec will be at
`openspec/specs/ascii85-decode-filter/spec.md` after `/opsx-archive`. The
delta operations below (`## ADDED Requirements`, `## MODIFIED
Requirements`) are merged into the canonical spec by the archiver.

## ADDED Requirements

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Concern — Delta specs omit the mandatory Status/Scope/OpenSpec changes header

The writing-specs.md guide prescribes that every spec file opens with **Status**, **Scope**, **OpenSpec changes**, ## Purpose, etc. headers. None of the 8 spec deltas include this. The guide does not explicitly say delta specs reuse the header, but the same content has to materialise when /opsx-archive produces canonical openspec/specs/<cap>/spec.md.

Suggested fix: Either (a) add a brief ## Purpose paragraph at the top of each delta spec.md, or (b) leave a TODO noting the archiver must add Purpose/Non-Functional/Acceptance/Notes when promoting.


### REQ-001: ASCII85Decode SHALL decode per PDF 1.7 §7.4.3

The decoder MUST interpret 5-character groups in the range `!..u` (codepoints 33..117) as base-85 integers, emit 4 bytes per group in big-endian order, recognise the single-character shortcut `z` as 4 zero bytes, terminate at the `~>` EOD marker, and ignore whitespace anywhere.

#### Scenario: Standard 5-char group decode

- GIVEN the input is `87cURD]i,"Ebo80~>` (encoding `"Hello world!"`)
- WHEN `ASCII85Decode` is invoked
- THEN the decoder MUST return `Hello world!`

#### Scenario: `z` shortcut

- GIVEN the input is `z~>`
- WHEN `ASCII85Decode` is invoked
- THEN the decoder MUST return `\x00\x00\x00\x00` (4 zero bytes)

#### Scenario: Whitespace ignored

- GIVEN the input is `87cURD\n]i,"E\tbo80~>`
- WHEN `ASCII85Decode` is invoked
- THEN the decoder MUST return `Hello world!` (whitespace stripped)

#### Scenario: Trailing partial group

- GIVEN the input is `87cURDZ~>` (5-char group `87cUR` decoding to 4 bytes `0x48 0x65 0x6C 0x6C` = `"Hell"`, followed by 2-char partial group `DZ` decoding to the single byte `0x6F` = `"o"` per §7.4.3 padding rules)
- WHEN `ASCII85Decode` is invoked
- THEN the decoder MUST return the 5-byte string `"Hello"` (concatenation `0x48 0x65 0x6C 0x6C 0x6F`)

### REQ-002: ASCII85Encode SHALL produce round-trip-compatible output

The encoder MUST emit 5 ASCII chars per 4 input bytes, use the `z` shortcut for aligned 4-zero-byte groups, terminate with `~>`, and handle trailing partial groups by padding with `u` and emitting only the relevant prefix.

#### Scenario: Basic encode

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Concern — Requirement headings lack REQ-NNN numbering

The writing-specs.md guide mandates ### REQ-{NNN}: <Name> numbering inside spec files so that tasks.md and PR titles can reference them as {capability}#REQ-NNN. All 49 requirements across the 8 spec.md files use descriptive ### Requirement: <SHALL-statement> headings.

Impact: tasks.md entries cannot use the canonical cross-reference shorthand. The current ### Requirement: ... heading collapses to a fragile GitHub anchor — renaming the requirement breaks every link.

Suggested fix: Renumber all requirements as ### REQ-001: <Name>, ### REQ-002: <Name> etc. within each spec.md.

- GIVEN the input is the string `Hello world!`
- WHEN `ASCII85Encode` is invoked
- THEN the encoder MUST return a stream ending in `~>` that round-trips to the input

#### Scenario: Aligned zero run uses `z`

- GIVEN the input is `\x00\x00\x00\x00` (4 zero bytes)
- WHEN `ASCII85Encode` is invoked
- THEN the encoder MUST return `z~>`

#### Scenario: Empty input

- GIVEN the input is the empty string
- WHEN `ASCII85Encode` is invoked
- THEN the encoder MUST return `~>` (EOD only)

### REQ-003: ASCII85Decode round-trip MUST be lossless

For any input byte string `$P`, `ASCII85Decode(ASCII85Encode($P, null), null)` MUST equal `$P` byte-for-byte.

#### Scenario: Binary round-trip

- GIVEN the input is `random_bytes(1024)`
- WHEN `ASCII85Decode` is invoked
- THEN the round-trip MUST equal the input byte-for-byte

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Concern — Vague THEN clause in 'Trailing partial group' scenario

Scenario says **WHEN** the input is \87cURDZ~>`/THEN the decoder MUST return the appropriate 5-byte string per §7.4.3 padding rules`. The THEN clause does not state what the expected 5-byte output IS — it defers to the spec. This is exactly the 'too vague' anti-pattern.

Suggested fix: Compute the expected output byte-by-byte and write it explicitly, e.g. **THEN** the decoder MUST return the 5-byte string \``.

### REQ-004: ASCII85Decode SHALL fail safely on illegal characters

Characters outside `!..u` plus `z` plus whitespace plus the EOD bytes `~>` MUST cause the decoder to call `p_error()` and return the raw input unchanged.

#### Scenario: Illegal character

- GIVEN the input is `87c{RD~>` (contains `{`, codepoint 123, outside the range)
- WHEN `ASCII85Decode` is invoked
- THEN the decoder MUST call `p_error()` and return the original input unchanged

## MODIFIED Requirements

### REQ-001: Array-form `/Filter` SHALL decode in forward chain order

The dispatcher MUST recognise `/ASCII85Decode` and route to the ASCII85Decode helper.

#### Scenario: ASCII85 outer + Flate inner

- WHEN an object has `/Filter [/ASCII85Decode /FlateDecode]` and `_stream` is `ASCII85Encode(gzcompress("BT...ET"))`
- THEN `get_stream(false)` MUST return `"BT...ET"` byte-for-byte

### REQ-002: Array-form `/Filter` SHALL encode in reverse chain order

The dispatcher MUST recognise `/ASCII85Decode` on the encode path and route to the ASCII85Encode helper.

#### Scenario: Two-filter encode round-trip with ASCII85

- WHEN `set_stream($P, false)` is called on `/Filter [/ASCII85Decode /FlateDecode]`, then `get_stream(false)` is called
- THEN the value returned by `get_stream(false)` MUST equal `$P`
50 changes: 50 additions & 0 deletions openspec/changes/feat-ascii85-decode/tasks.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
## 1. Pre-flight

- [ ] 1.1 Confirm `feat-filter-chain-dispatch` is merged into `work/text-replacement`
- [ ] 1.2 Branch off as `feat/ascii85-decode`
- [ ] 1.3 Re-read PDF 1.7 §7.4.3 — confirm `z` shortcut + EOD + partial-group padding semantics

## 2. Decode helper

- [ ] 2.1 Add `protected static function ASCII85Decode($_stream, $params)` in `src/PDFObject.php`
- [ ] 2.2 Strip whitespace, optionally strip leading `<~` (OQ1 — Adobe-tolerant)
- [ ] 2.3 Process `z` shortcut as 4 zero bytes
- [ ] 2.4 Process 5-char groups as base-85 integers, big-endian to 4 bytes
- [ ] 2.5 Handle trailing partial group with `u` padding
- [ ] 2.6 Fail-safe on illegal characters via `p_error()`

## 3. Encode helper

- [ ] 3.1 Add `protected static function ASCII85Encode($_stream, $params)`
- [ ] 3.2 Walk input 4 bytes at a time; emit `z` for aligned 4-zero-byte groups (D3)
- [ ] 3.3 Emit standard 5-char groups otherwise
- [ ] 3.4 Handle trailing partial group (pad with `\x00` bytes, emit only `k+1` chars where `k` is the partial length)
- [ ] 3.5 Emit trailing `~>` EOD marker

## 4. Chain-dispatch integration

- [ ] 4.1 Add `case '/ASCII85Decode'` arms in both `apply_filter_chain_decode` and `apply_filter_chain_encode`

## 5. Tests / verification

- [ ] 5.1 Round-trip fixture `examples/poc-filter-roundtrip-ascii85.php`: 1024-byte buffer including 4-zero-byte runs (exercises `z` shortcut)
- [ ] 5.2 Verify `examples/poc-replace-text.php` still exits 0
- [ ] 5.3 Negative test: illegal character → `p_error` + unchanged
- [ ] 5.4 Chain test `[/ASCII85Decode /FlateDecode]` round-trip
- [ ] 5.5 Adobe-tolerance test: input with leading `<~` decodes correctly

## 6. Upstream-PR draft

- [ ] 6.1 Update `docs/upstream-prs/03-ascii85-decode/{proposal,design,tasks}.md`
- [ ] 6.2 Leave `Posted at: <pending>` placeholder

## 7. Quality gate

- [ ] 7.1 PHP 7.4 compatibility; verify int is 32-bit-or-wider on target platform
- [ ] 7.2 No new composer dependencies
- [ ] 7.3 snake_case discipline

## 8. Commit + PR

- [ ] 8.1 Commit on `feat/ascii85-decode`
- [ ] 8.2 Open PR `feat/ascii85-decode → work/text-replacement`
2 changes: 2 additions & 0 deletions openspec/changes/feat-asciihex-decode/.openspec.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
schema: spec-driven
created: 2026-05-26
Loading