🐛 Suppress OPF-014 in single-file validation modes and other issues - #23
Merged
Conversation
OPF-014 compares the features a content document uses against the properties its manifest item declares. `checkSingleFile` never sets context.packageDocument, so `manifestItem?.properties?.includes(...)` was falsy for every feature and each one was reported as undeclared — a spurious ERROR that also flipped the CLI exit code. Measured against EPUBCheck 5.3.0 over Java's own standalone fixtures: 92 of 741 `--mode xhtml` files and 3 of 52 `--mode svg` files emitted a bogus OPF-014; Java emits none. Java guards the same checks with `context.container.isPresent()` (OPSHandler30.checkProperties), which this mirrors via a hasContainer() predicate. The paired OPF-015/OPF-018 branches need no guard: they require manifestItem to be truthy, so they already no-op in single-file mode. Standalone-mode agreement with Java, before → after: --mode xhtml 84.6% → 95.6% exact, 89.2% → 97.9% verdict --mode svg 82.7% → 88.5% exact, 86.5% → 92.3% verdict all modes 86.3% → 92.9% exact, 92.5% → 97.7% verdict
Two entangled changes — both touch opf/parser.ts and checker.ts, so they
cannot be separated into commits that each build.
--- Stop validating an unusable publication ---
Three defects, all from the pipeline running against a publication it
cannot meaningfully validate:
1. No abort when container.xml declares no Package Document. Java stops
after RSC-003 (OCFChecker.checkDeclaredPackageDocuments returns
false); we continued into OPFValidator and SchemaValidator, adding a
spurious FATAL OPF-002 and an RSC-005. runPipeline now returns early.
OPF-002 still fires for a *declared but missing* OPF, a different case
Java does report.
2. No abort when the package element declares no version. Java peeks the
version before any OPF checking (PackageDocumentPeekerHandler) and
aborts; parseOPF instead silently defaulted to 3.0, so an OEBPS 1.2
package was validated as EPUB 3 — 1 Java message became 12.
3. Version-dependent container checks used the *requested* version
(default 3.3) rather than the declared one, because container.xml is
parsed before the OPF. PKG-013 therefore only fired when the caller
passed `{version: '2.0'}` explicitly — which every EPUB 2 test does,
hiding the bug from the suite while real CLI users hit it. The
multiple-renditions branch (RSC-019/RSC-017) had the inverse fault and
fired when it should not. Both now run after the version is peeked,
matching OCFChecker.check.
peekOpfVersion distinguishes "no version attribute" (stop) from
"unreadable" (continue, so encoding and well-formedness errors are still
reported). matchPackageElement is shared with parseOPF so the
version/unique-identifier extraction is not duplicated.
--- Line numbers on package document messages ---
Only 34% of our messages carried a line number; Java manages 82%. The
OPF is parsed with regexes, and the model types had nowhere to record a
position, so every package-document message pointed at a file with no
line.
Section parsing now tracks each section's offset in the original source
and resolves it through a line index (src/util/location.ts). Comments are
blanked rather than stripped inside sections so offsets stay aligned with
the source. ManifestItem, SpineItemRef, DCElement, MetaElement and
LinkElement each carry the line they were declared on.
OPF-097 additionally now points at the manifest declaration rather than
the resource file, which is where Java points.
Line coverage across the 758-fixture corpus: 34.2% → 52.4%.
--- Fixture agreement with EPUBCheck 5.3.0 ---
ocf-metainf-container-mediatype-invalid 3 msgs → 1 (RSC-003), matches
opf-legacy-oebps12-error 12 msgs → 1 (OPF-001), matches
ocf-metainf-container-multiple-opf 3 msgs → PKG-013, matches
Corpus-wide, error/warning agreement 84.7% → 85.5%, verdict agreement
95.1% → 95.3%.
There was a problem hiding this comment.
Pull request overview
This PR aligns several validation behaviors with Java EPUBCheck by (a) avoiding OPF property-mismatch messages when validating standalone content files, (b) improving message source locations by tracking OPF element line numbers, and (c) peeking the OPF-declared publication version earlier to prevent downstream cascades when the OPF is unusable.
Changes:
- Suppress OPF-014 property checks when validating in single-file modes (no Package Document / manifest context).
- Track 1-based line numbers for key OPF elements and use them to point manifest-related messages (e.g., OPF-097) at the OPF declaration site.
- Peek and resolve the declared OPF version earlier (container stage + pipeline guard) to match Java’s validation flow.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/util/location.ts | Adds helpers for building EPUBLocation safely and indexing source offsets to 1-based line numbers. |
| src/references/validator.ts | Uses declaredAt (when available) so OPF-097 points at the manifest declaration. |
| src/references/types.ts | Extends Resource with declaredAt?: EPUBLocation for declaration-site reporting. |
| src/opf/types.ts | Adds optional line?: number to OPF element types for location tracking. |
| src/opf/parser.ts | Adds peekOpfVersion(), introduces comment-blanking section extraction with offsets, and populates line fields during parsing. |
| src/ocf/container.ts | Exports OPF_MEDIA_TYPE and peeks OPF version during container parsing to apply version-conditional checks correctly. |
| src/content/validator.ts | Gates OPF-014 “missing property” messages behind presence of a Package Document in single-file modes. |
| src/checker.ts | Uses OPF_MEDIA_TYPE, stores manifest declaration locations, and adds a pipeline guard to resolve/require a usable publication version. |
Comment on lines
+78
to
+82
| export function peekOpfVersion(xml: string): OpfVersionPeek { | ||
| const { version } = matchPackageElement(xml); | ||
| if (version) return { kind: 'declared', version }; | ||
| return /<package[\s>]/.test(xml) ? { kind: 'undeclared' } : { kind: 'unreadable' }; | ||
| } |
Comment on lines
+489
to
+500
| /** | ||
| * Whether a Package Document is available to compare content features against. | ||
| * | ||
| * OPF-014/015/018 report a mismatch between the features a content document uses | ||
| * and the properties its manifest item declares. In single-file mode there is no | ||
| * manifest, so the comparison is meaningless and every detected feature would be | ||
| * reported as undeclared. Java guards the same checks with | ||
| * `context.container.isPresent()` (OPSHandler30.checkProperties). | ||
| */ | ||
| private hasContainer(context: ValidationContext): boolean { | ||
| return context.packageDocument !== undefined; | ||
| } |
01e61c9 fetches five Project Gutenberg titles into test/fixtures/real/ and describes them as living in "a gitignored cache", but no ignore rule was ever added. The ~26MB of .epub files have been sitting untracked since, one `git add -A` away from being vendored into the repo the commit was explicitly trying to keep small.
be1fa7e and 0db5503 added three behaviours with no direct coverage: peekOpfVersion, the line-offset tracking in parseOPF, and the container guard that suppresses OPF-014 when there is no Package Document. peekOpfVersion is pinned across all three arms of its return union — declared (both attribute orderings), undeclared, and unreadable. The last two are what the call sites branch on differently, so collapsing them would be silent. Line numbers are asserted against a fixture built as a line array, so every expected number is checkable by eye. A multi-line comment sits above the asserted elements: comment blanking preserves newlines precisely so it cannot shift them, and nothing else guarded that. The single-file tests assert each feature twice — clean in single-file mode, and reported with the offending property named inside a container whose manifest item omits it. Without the paired half the suppression tests would pass just as happily if feature detection stopped working. Verified by reverting each fix in turn: dropping the container guard fails 5, using section-relative offsets fails 3, and dropping newlines from comment blanking fails 2. 1361→1383 passing.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.