[0.2.1] RC | Preflight & GUI Foundation

[mberrys]

No description provided.

[mberrys]

@codex

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a89ed56646

ℹ️ 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".

[chatgpt-codex-connector]

Honor the explicit transparency opt-out

For PDF/X-1a and PDF/X-3 conversions, an explicit flatten_transparency: false is stored as settings.flattenTransparency == false, but this || flattensTransparencyByDefault(...) immediately turns it back on. Consequently the preview claims transparency will be flattened and apply() rasterizes page content despite the documented opt-out; the setting needs an explicit auto/true/false distinction or the parsed value must be used directly.

Useful? React with 👍 / 👎.

[mberrys]

Fixed in 2eb00de. The parsed flatten_transparency value is now a three-state policy (Automatic / Always / Never) resolved once by flattensTransparency(), so an explicit false is no longer re-enabled by the target default: preview() stops advertising a transparency.flatten change and apply() does not run the flattener. An explicit opt-out on a document with live transparency is fail-closed — pdfx.transparency.allowed stays a blocker. PDFStandardConversionSettings::flattenTransparency was added by this same PR and has never shipped in unstable/stable, so replacing it is not a break for a released consumer.

[mberrys]

Worth recording alongside this fix: while verifying it I found that the PDF/X rules were not running at all. pdfxProfile() returned a profile with no checks array, and PreflightEngine::parseProfile() (preflightengine.cpp:6110-6115) rejects an empty checks array before it reads pdfx. So preview() reported zero blockers for every PDF/X target — making collectPreflightBlockers() dead code and the pdfx.transparency.allowed fixability clause unreachable — and every PDF/X apply() failed at postflight. Fixed here in 08d42b9 by giving the profile the shared checks of loop-preflight/examples/profile-pdfx-x1a2001.json; this is what makes the transparency opt-out actually fail closed rather than silently succeeding. Recorded as #556.

[chatgpt-codex-connector]

Consolidate the change fragments

This commit adds eight separate changes/*.md fragments, whereas repository policy requires every PR to add exactly one. Consolidate these entries into the single fragment representing this change so changelog attribution and change validation remain deterministic.

AGENTS.md reference: AGENTS.md:L35-L35

Useful? React with 👍 / 👎.

[mberrys]

Not applicable to this PR, and no fragment is removed.

The eight fragments this PR carries are the per-topic-PR fragments of the PRs already merged into dev; each of those PRs added exactly one fragment named after its own head branch. For this promotion PR the changelog check is explicitly not-applicable, because the head branch (dev) is an integration branch:

$ python scripts/agent/check-change.py --base origin/unstable --head origin/dev --head-branch dev --dry-run
  →  changelog: {"result": "not-applicable", "reason": "integration branch"}

scripts/agent/test_check_change.py::test_check_changelog_allows_stacked_topic_fragments already asserts that stacked topic fragments are legal, and deleting them here would falsify the changelog attribution of those merged PRs. changes/dev.md (this PR's own fragment) was updated in 7bd2363 to cover the review fixes.

[chatgpt-codex-connector]

Assign the summary before emitting its notify signal

When a completed preflight becomes stale, setState() emits stateChanged before m_operatorSummary is updated, even though that signal is the operatorSummary property's notifier and is also what triggers EditorHost::presentationChanged. Observers handling the signal can therefore read and retain the previous run's summary; cancelRun() has the same ordering. Set the summary first or emit a dedicated notification after changing it.

Useful? React with 👍 / 👎.

[mberrys]

Fixed in 22225d1. setCurrentRevision() and cancelRun() now assign m_operatorSummary before calling setState(), so an observer reading the summary from the stateChanged handler never sees the previous run's copy. The new slot operatorSummaryIsCurrentWhenTheStateSignalFires reads the summary from inside the signal handler and fails on the unmodified code ("Preflight is running." vs the expected stale/cancelled text).

[chatgpt-codex-connector]

Publish the outcome only after commit succeeds

With safeWrite == true, the nested device overload writes Appended or CopiedUnchanged directly into the caller's outcome before QSaveFile::commit() runs. If the final rename or flush fails, this overload returns an error while leaving an outcome that claims the save completed, contradicting the API's “set on success” contract. Hold the nested outcome locally and copy it to the caller only after a successful commit.

Useful? React with 👍 / 👎.

[mberrys]

Fixed in c2c7373. The fileName overload now holds the nested IncrementalWriteOutcome locally and copies it to the caller only after QSaveFile::commit() succeeds (and, in the direct-write branch, only after close()).

One caveat, stated plainly: on this platform the failure path cannot be covered by a test. QSaveFile::open() refuses a read-only target ("Existing file ... is not writable") before the nested write runs, so no test can reach commit() and be made to fail for the intended reason. That is recorded in the commit message rather than papered over with a test that cannot fail. The new slot fileOverloadReportsWhatItDid covers the file overload's success reporting (append vs verbatim copy), which was previously untested; it passes identically before and after this change.

[chatgpt-codex-connector]

Clear the interactive-thread registration on host teardown

EditorHost registers its construction thread globally, but its destructor never calls clearInteractiveThread(). When a host is destroyed while the process continues—such as an embedded or recreated editor—later synchronous preflight work on that thread is still rejected as an interactive-thread violation, and a host created on a temporary thread leaves a stale QThread* registration. Pair this registration with lifetime-aware cleanup.

Useful? React with 👍 / 👎.

[mberrys]

Fixed in 057cba7. EditorHost::~EditorHost() now clears the registration when the destroying thread is the one that registered it, so a recreated or embedded host no longer leaves a stale registration that keeps refusing synchronous blocking work on that thread.

[chatgpt-codex-connector]

Flatten before the CMYK normalization pass

For a typical non-spot PDF/X-1a conversion, both color normalization and transparency flattening are enabled, but the new flattening call runs after PDFRgbToCmykFixup. The flattener's non-spot path embeds its QImage page raster as DeviceRGB, reintroducing exactly the color space the earlier normalization removed; the independent validator or internal pdfx.color.device-rgb postflight then rejects the candidate. Run flattening before RGB-to-CMYK or emit a CMYK raster.

Useful? React with 👍 / 👎.

[mberrys]

Deferred, not dismissed. Reordering alone cannot fix this.

PDFRgbToCmykFixup refuses to convert RGB image XObjects — LoopLibCore/sources/pdfrgbtocmykfixup.cpp:462-479 appends every image whose /ColorSpace resolves to an RGB name to report->unsupported with the reason "RGB image XObjects require image-sample conversion", and both preview() (blockers) and apply() (early return) treat that as a hard failure. The flattener's non-spot path embeds exactly such a DeviceRGB page raster (pdftransparencyflattener.cpp:384-416).

So with today's order (CMYK, then flatten) a working postflight rejects the DeviceRGB candidate; with the order you propose (flatten, then CMYK) the CMYK pass fails first with "RGB image XObjects require image-sample conversion". Neither order lets a PDF/X-1a conversion of a document with live transparency succeed.

Resolving it needs either image-sample conversion in the fixup (no API change, but it changes behaviour for every consumer) or a CMYK raster from the flattener (which changes the exported PDFTransparencyFlattenSettings, i.e. a protected interface). Filed as #555 and deliberately left out of this review-fix PR.

[chatgpt-codex-connector]

Translate the operator-facing verdict summaries

preflightVerdictOperatorSummary() now supplies the text displayed directly by PreflightPane.qml, but its pass and fallback messages use QStringLiteral rather than a translation context. In every non-English build, a clean, incomplete, or otherwise reason-less run therefore replaces the previously translatable QML status with English-only copy. Route these operator-facing strings through QCoreApplication::translate or the Core translation context.

Useful? React with 👍 / 👎.

[mberrys]

Fixed in ed4314f. preflightVerdictOperatorSummary() now routes every literal through QCoreApplication::translate("pdf::PreflightVerdict", ...) — the same context preflightGateFailureMessage() already used — so PreflightPane.qml is no longer English-only. The literals are byte-identical, which is why the pre-existing untranslated-path assertions still pass. The test binary needed QTEST_GUILESS_MAIN instead of QTEST_APPLESS_MAIN for a QTranslator to be installable at all; that macro change is in the same commit.

[chatgpt-codex-connector]

Carry waiver state into the findings model

When all error findings have active dispositions, the reducer returns Pass here, but the controller has already copied the original error findings into PreflightFindingsModel without any decision or waived-state information. The Editor consequently announces a passing verdict while its findings list and canvas overlays continue to expose those entries as ordinary errors. Pass the active dispositions into the model so waived findings receive their distinct presentation instead of contradicting the overall state.

Useful? React with 👍 / 👎.

[mberrys]

Fixed in 82e4f43. acceptResult() now passes verdict.waivedFindingIds into PreflightFindingsModel::replace(), which marks the affected views (waived, exposed as WaivedRole) and maps them to a non-blocking overlay severity, so the canvas stops painting a PASS run's findings as blocking errors. The dispositions now reach the same model the verdict was reduced from, so the list and the headline can no longer contradict each other.

[chatgpt-codex-connector]

Preserve the exported incremental-writer overloads

Adding a defaulted parameter changes the mangled C++ symbol; it does not preserve the previous four-argument exported overload. Existing binaries built against LoopLibCore will therefore fail to resolve writeIncremental(...) after updating to this library even though the change is marked non-breaking. Keep the old signature as a forwarding overload and add the outcome-reporting overload separately.

AGENTS.md reference: AGENTS.md:L37-L37

Useful? React with 👍 / 👎.

[mberrys]

Fixed in 8ddc6e0. Both four-argument overloads are back as real forwarding functions and both default arguments are gone, so the original mangled symbols are preserved and a four-argument call stays unambiguous (a default argument plus an overload would have made every such call ambiguous).

Symbol check (dumpbin is not installed here; this is the DLL's PE export table): LoopLibCore.dll exports exactly four writeIncremental symbols — the two four-argument signatures plus the two five-argument ones. UnitTestsIncrementalSave now static_casts both signatures, so a future defaulted parameter cannot silently remove them again.

[chatgpt-codex-connector]

Align the OCR language schema with runtime normalization

The service deliberately trims and lowercases language values before validating them, so a request containing "EN" is accepted and normalized to "en" (the added test explicitly expects this). The tightened protocol schema, however, rejects the same request because its pattern permits only lowercase input. Clients validating requests against the published schema will reject inputs the runtime contract accepts; either make the schema case-insensitive in shape or stop accepting uppercase at runtime.

Useful? React with 👍 / 👎.

[mberrys]

Fixed in b6832f3. The schema pattern is now case-tolerant in shape (^[A-Za-z]{2,3}(_[A-Za-z]{2,4})?$, plus a description saying the service normalizes case) instead of lower-case only, so a client validating the published contract no longer rejects a request the runtime accepts and normalizes. The new test checks the schema and the runtime agree in both directions: the codes the runtime normalizes must match the pattern, and the traversal-shaped junk it refuses must not.