Skip to content

fix(ffp): unconflict #126 onto main and repair the errors hiding behind its syntax fix (#122) - #127

Merged
hyperpolymath merged 5 commits into
mainfrom
arena/01a108e5-presswerk
Oct 4, 2026
Merged

hyperpolymath merged 5 commits into
mainfrom
arena/01a108e5-presswerk

Conversation

@hyperpolymath

Copy link
Copy Markdown
Owner

Fixes the conflicted merge in #126, and completes the CI repair for #122.

1. The merge conflict, resolved

#126's branch still carries fa14610 — the same tree that reached main as the #125 merge commit (eb98112) — so GitHub cannot three-way-merge it (CONFLICTING). fa14610's tree is byte-identical to main's tip, so this branch replays #126's three unique commits cleanly on top of main (verified: git diff fa14610 origin/main is empty; the cherry-picks applied without a single conflict). This PR's final tree is therefore #126's intended end-state, rebased.

Because this session's branch is arena/01a108e5-presswerk, the rebased result lives here rather than on #126's branch; this PR supersedes #126, which should be closed.

2. What #126's syntax fix unblocked — and what this PR repairs on top

The syntax error in presswerk-document/src/provenance.rs had stopped rustc at parse stage, so nothing behind it was ever compiled or tested. After applying #126's fixes, a line-by-line audit against the pinned lopdf 0.40 source found and fixed:

Compile errors

  • object_to_string matched Object::Integer64 — a variant that does not exist in lopdf 0.40 (Integer already carries i64). Removed.

Test bugs (fixtures shaped unlike real viewer output)

  • The "hand/viewer-filled" fixtures in the detector unit tests and the IPP-server tests used /AP << /N null >>. The detector counts that as a missing appearance, so viewer_filled_is_unknown would see Incomplete where it asserts Generated, and the blank-print hazard (appearances == incomplete && filled > 0) would hold the hand-filled job that hold_policy_leaves_hand_filled_form_alone expects to stay Pending. Both fixtures now carry a real appearance stream — the shape of the conformance viewer-filled vector.

Appearance semantics

  • /N resolution rewritten (state_appearance_exists): a stream is an appearance; a state-keyed dictionary requires the current state (/AS, falling back to a name /V) to select an entry. This also removes an unread n_resolved binding from a match whose arms all return.

Clippy cleanliness (CI runs clippy --workspace --all-targets -- -D warnings)

  • Removed unused variables: xmp_found, xmp_unreadable, payload_s (detector), is_machine_or_suspected (jobs page).
  • walk_field had nine parameters plus a never-written evidence parameter → now takes an Inherited { ft, ff, v } struct (six parameters).
  • .map(|o| object_to_string(o)) → .map(object_to_string); dropped a redundant else after a diverging if, the duplicated pushbutton parse + dead parse_ff, and the dead resolve_array helper.
  • FfpClassification::from_str carries #[allow(clippy::should_implement_trait)] with a comment: the token vocabulary deliberately never fails to parse (unknown → Unreadable), mirroring from_token.
  • row_to_print_job: collapsed two identical if branches.

Misc

  • The %PDF- pre-check in classify_document was an empty if block; it now does what its comment promised (returns unreadable when raw bytes are present and lack the header). No vector outcome changes — classify_pdf already failed such input at lopdf::load_mem.
  • Fixed a doc comment in pdf/reader.rs containing leaked literal \n escapes.

3. Conformance status (#122, criterion 2)

All 14 FFP v1.0.0 conformance vectors were traced by hand through the detector against probe.awk and make-fixtures.sh in hyperpolymath/standards (spec now merged to main, 1-formats/sub-specs/form-fill-provenance/spec/conformance/): each vector's expected canonical line matches the detector's code path, including the negative controls (blank-form-need-appearances → blank-form, viewer-filled → filled-unknown, unreadable → unreadable via load_mem failure). just ffp-conformance (FFP_DETECTOR=./target/debug/ffp-classify bash …/run-conformance.sh) runs the real 14/14 check wherever a standards checkout is available.

Resolves #122
Supersedes #126

hyperpolymath and others added 4 commits October 4, 2026 21:52
…heck)

- presswerk-core/src/provenance.rs: From<FormProvenanceLegacy> used moved
  legacy.marker/producer twice and checked after move -> use & + clone
  to avoid E0382 (clippy -D warnings).
- presswerk-document/src/provenance.rs: get_array_ids had syntax
  error 'Object::Reference kid)' and impossible
  'Object::Reference(id)=resolve_reference(..)' (type mismatch) and
  extra closing delimiter at 770 -> rewrite to simple loops, remove
  dead resolve_reference helper.

Both were reported as cargo check failures in CI (Test + rust-ci).

Co-authored-by: arena-agent <297053741+arena-agent@users.noreply.github.com>
- fmt.yml runs cargo fmt --all on PR and pushes fix
- updated actions.lock to include fmt.yml (same 3 actions as ci.yml)

Co-authored-by: arena-agent <297053741+arena-agent@users.noreply.github.com>
- ci.yml: run cargo fmt --all before Verify so PRs don't fail on fmt
- rust-ci.yml: add fmt job (checkout + toolchain + cache + cargo fmt --all + push) before reusable Rust CI, needs: fmt
- actions.lock: add rust-ci.yml entry (same 3 actions as ci.yml) + keep fmt.yml entry

This lets the PR's fmt be fixed in-place without local cargo, and
lets Rust CI's cargo fmt --check pass after the fmt job pushes.

Co-authored-by: arena-agent <297053741+arena-agent@users.noreply.github.com>
…e syntax error

PR #126's syntax fix unblocked parsing of presswerk-document/src/provenance.rs,
but everything behind that parse error had never been compiled or tested. This
repairs what surfaces next:

Compile errors (verified against lopdf 0.40 source, the pinned tag):
- object_to_string: drop the Object::Integer64 arm; lopdf 0.40 has no such
  variant (Integer already carries i64).

Test bugs (fixtures shaped unlike real viewer output):
- viewer/hand-filled fixtures in the detector unit tests and the IPP server
  tests used /AP << /N null >>. The detector counts that as a missing
  appearance, so viewer_filled_is_unknown would see Incomplete where it
  asserts Generated, and the blank-print hazard would hold the hand-filled
  job that hold_policy_leaves_hand_filled_form_alone expects Pending. Give
  those fixtures a real appearance stream, matching the conformance
  viewer-filled vector shape.

Appearance handling:
- /N resolution rewritten: a stream is an appearance; a state-keyed
  dictionary requires the current state (/AS, falling back to a name /V)
  to select an entry (state_appearance_exists). Removes the unread
  n_resolved binding from a match whose arms all return.

Clippy cleanliness (CI runs clippy -D warnings):
- remove unused variables (xmp_found, xmp_unreadable, payload_s in the
  detector; is_machine_or_suspected in jobs.rs)
- walk_field: nine parameters plus a never-written evidence parameter ->
  Inherited { ft, ff, v } struct, six parameters
- .map(|o| object_to_string(o)) -> .map(object_to_string)
- drop redundant else after diverging if, the duplicated pushbutton parse
  and dead parse_ff, and the dead resolve_array helper
- FfpClassification::from_str: allow should_implement_trait; the token
  vocabulary never fails to parse (unknown -> Unreadable), by design
- row_to_print_job: collapse identical if branches

Also make the %PDF- pre-check do what its comment promised (it was an empty
block) and fix a doc comment with leaked literal \n escapes.

Co-authored-by: arena-agent <297053741+arena-agent@users.noreply.github.com>
@coderabbitai

coderabbitai Bot commented Oct 4, 2026 •

Copy link
Copy Markdown

Important

  • 🔍 Trigger review

This repository does not receive automatic reviews because it has fewer than 10 stars.

⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: b8b8cbdb-5451-4679-ae88-422bd111c746
  • Autopilot · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@hyperpolymath
hyperpolymath merged commit f8e745d into main Oct 4, 2026
5 checks passed
@hyperpolymath
hyperpolymath deleted the arena/01a108e5-presswerk branch October 4, 2026 22:11
hyperpolymath added a commit that referenced this pull request Oct 5, 2026
…ckfile drift (#128)

Follow-up to #127 (resolves #122). The first full CI compile of the
merged FFP stack surfaced three blockers; this fixes all of them.

## 1. `presswerk-document` compile failure (E0599) — pre-existing,
unmasked
`writer.rs` still used printpdf **0.8** ops `Op::SetFontSizeBuiltinFont`
/ `Op::WriteTextBuiltinFont`, which upstream removed when printpdf was
bumped to **0.9.1** back in #20 (May 2026). The breakage stayed latent
because `presswerk-core` failed to compile first; repairing core in
#125/#127 unmasked it. Migrated to the 1:1 PDF operators documented as
their replacement in printpdf 0.9:
- `Op::SetFont { font: PdfFontHandle::Builtin(BuiltinFont::Helvetica),
size }` (Tf)
- `Op::ShowText { items }` (Tj/TJ)

## 2. clippy `manual_map` in the FFP legacy→v1 bridge
`presswerk-core/src/provenance.rs` — `else if let Some(p) =
&legacy.producer { Some(…) } else { None }` rewritten as
`legacy.producer.as_ref().map(…)` per clippy's own suggestion. The E0382
repair is preserved (borrow + clone, no moves).

## 3. CI plumbing
- **rust-ci.yml**: removed the `fmt` job added by #126 and restored the
wrapper-only form — its non-empty `actions.lock` section caused
`startup_failure` (0 jobs) on every run since the merge. Auto-formatting
remains covered by the standalone `fmt.yml` workflow (proven: it pushed
the fmt commit on #127).
- **actions.lock** drift reconciliation:
- `fmt.yml`: canonical lowercase `swatinem/rust-cache@v2.9.2` key
(matches `dependencies:` + all other sections)
- `github/codeql-action` v4.38.0 → **v4.38.2** (workflow bumped by
dependabot in #121 without a relock — CodeQL has been `startup_failure`
since)
  - `haskell-actions/setup` v2.12.0 → **v2.12.1** (casket-pages drift)
- `rust-ci.yml` section restored to `[]` (estate convention for
reusable-call wrappers, cf. `governance.yml`)

> The Governance **Actions lockfile verify** failure predates this work
(every main push since mid-September). This PR fixes the drifts
identifiable without the `gh-actions-lock` tool; deeper reconciliation
needs that tooling.

No Rust toolchain is reachable from this workspace, so verification is
by careful API audit against printpdf v0.9 sources plus CI on this PR.

---------

Co-authored-by: arena-agent <297053741+arena-agent@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(ffp): classify and record machine-filled form provenance in the print path (D189)

1 participant