Conversation
WalkthroughAdds two WCAG 2.2 PDF‑2.0 flavour constants (human/machine), introduces PDFAFlavours.PDF_2_0_PART, adjusts Specification mappings (WCAG_2_2 → ISO_32000_1_7; new WCAG_2_2_PDF_2_0 → ISO_32000_2_0), and updates flavour ID construction plus PDF‑UA related checks. Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant Flavour as PDFAFlavour
participant Const as PDFAFlavours
Caller->>Flavour: getID()
alt standard == WCAG_2_2_PDF_2_0
Flavour->>Const: read PDF_2_0_PART ("2")
Flavour-->>Caller: return baseID + "-" + "2"
else other standard
Flavour-->>Caller: return baseID
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
core/src/main/java/org/verapdf/pdfa/flavours/PDFAFlavour.java(2 hunks)core/src/main/java/org/verapdf/pdfa/flavours/PDFFlavours.java(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
core/src/main/java/org/verapdf/pdfa/flavours/PDFAFlavour.java (1)
core/src/main/java/org/verapdf/pdfa/flavours/PDFAFlavours.java (1)
PDFAFlavours(34-116)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (1)
core/src/main/java/org/verapdf/pdfa/flavours/PDFFlavours.java (1)
49-52: UA1/UA2 mapping logic is consistent with the new WCAG 2.2 PDF 1.7 / 2.0 split, but sensitive to flavour-id clashesThe revised helpers look logically sound:
isPDFUA1RelatedFlavournow treats:
PDFUA_1itself, plus- WCAG flavours whose part is
Specification.WCAG_2_1orSpecification.WCAG_2_2(your PDF 1.7 WCAG 2.2 spec)
as UA1-related.
isPDFUA2RelatedFlavournow treats:
PDFUA_2,Specification.WTPDF_1_0, andSpecification.WCAG_2_2_PDF_2_0
as UA2-related.Using
isFlavourPart(flavour, spec)(enum identity onSpecification) keeps these checks precise and future-proof against additional WCAG family members.However, because the WCAG 2.2 1.7 and 2.0 flavours currently share the same string ids (
"wcag2h"/"wcag2m") inPDFAFlavour, a flavour obtained viabyFlavourId("wcag2h")will be recognized as UA2-related (PDF 2.0 spec), whilePDFAFlavour.WCAG_2_2_HUMANis UA1-related (PDF 1.7 spec). Once the flavour-id collision inPDFAFlavouris resolved, these helpers should behave as intended for both enum-based and string-based flows.I’d recommend adding/adjusting tests to cover:
- UA1 vs UA2 classification for:
PDFUA_1,PDFUA_2,- WCAG 2.1,
- WCAG 2.2 (PDF 1.7),
- WCAG 2.2 PDF 2.0, and WTPDF 1.0.
- The behaviour when flavours are sourced via
PDFAFlavour.byFlavourId/fromStringrather than direct enum references.Also applies to: 65-66
MaximPlusov
force-pushed
the
wcag_2_0
branch
2 times, most recently
from
fd7753f to
94334f4
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
core/src/main/java/org/verapdf/pdfa/flavours/PDFAFlavour.java (1)
220-226: Specification id and name duplication between WCAG_2_2 and WCAG_2_2_PDF_2_0.Both
Specification.WCAG_2_2andSpecification.WCAG_2_2_PDF_2_0share the samepartNumber(2),subpartNumber(2),year, anddescription. As a result,calculateID()produces the identical id"WCAG2.2"for both, and thenamefield is"WCAG-2"for both.If any code, logging, configuration, or UI relies on
Specification.getId()orgetName()as unique identifiers, this collision will cause ambiguity — users or systems won't be able to distinguish the PDF 1.7 variant from the PDF 2.0 variant.Consider one of these approaches:
- Use a distinct subpart or series for the PDF 2.0 variant to differentiate at the specification level.
- Override or extend
calculateID()to include a PDF version marker for specifications that have multiple baseline PDF versions (e.g., append"-pdf17"or"-pdf20").- Introduce an explicit
displayNameoruniqueIdfield ifidandnamemust remain the same for compatibility.For example:
WCAG_2_2(IsoStandardSeries.NO_SERIES, PDFSpecification.ISO_32000_1_7, SpecificationFamily.WCAG, PDFAFlavours.WCAG_2_2_PART, PDFAFlavours.WCAG_2_2_SUBPART, PDFAFlavours.WCAG_2_2_YEAR, PDFAFlavours.WCAG_2_2_DESCRIPTION), -WCAG_2_2_PDF_2_0(IsoStandardSeries.NO_SERIES, PDFSpecification.ISO_32000_2_0, SpecificationFamily.WCAG, - PDFAFlavours.WCAG_2_2_PART, PDFAFlavours.WCAG_2_2_SUBPART, PDFAFlavours.WCAG_2_2_YEAR, - PDFAFlavours.WCAG_2_2_DESCRIPTION); +WCAG_2_2_PDF_2_0(IsoStandardSeries.NO_SERIES, PDFSpecification.ISO_32000_2_0, SpecificationFamily.WCAG, + PDFAFlavours.WCAG_2_2_PART, PDFAFlavours.WCAG_2_2_PDF_2_0_SUBPART, PDFAFlavours.WCAG_2_2_YEAR, + PDFAFlavours.WCAG_2_2_DESCRIPTION);Where
WCAG_2_2_PDF_2_0_SUBPARTcould be a distinct value that produces a unique id like"WCAG2.2-pdf2".
🧹 Nitpick comments (1)
core/src/main/java/org/verapdf/pdfa/flavours/PDFAFlavour.java (1)
131-135: ID collision successfully resolved with postfix logic.The postfix logic correctly differentiates WCAG 2.2 PDF 1.7 and PDF 2.0 variants:
wcag2h/wcag2mfor PDF 1.7wcag2h2/wcag2m2for PDF 2.0This resolves the
FLAVOUR_LOOKUPcollision from the previous review.The current approach hardcodes the check for
WCAG_2_2_PDF_2_0. If additional PDF 2.0 variants are introduced for other WCAG versions, consider extracting the postfix logic into a method that checks a specification property (e.g.,requiresPdfVersionPostfix()), or use a naming convention in the specification enum to drive the postfix automatically.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
core/src/main/java/org/verapdf/pdfa/flavours/PDFAFlavour.java(3 hunks)core/src/main/java/org/verapdf/pdfa/flavours/PDFAFlavours.java(1 hunks)core/src/main/java/org/verapdf/pdfa/flavours/PDFFlavours.java(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- core/src/main/java/org/verapdf/pdfa/flavours/PDFAFlavours.java
🧰 Additional context used
🧬 Code graph analysis (1)
core/src/main/java/org/verapdf/pdfa/flavours/PDFAFlavour.java (1)
core/src/main/java/org/verapdf/pdfa/flavours/PDFAFlavours.java (1)
PDFAFlavours(34-117)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (2)
core/src/main/java/org/verapdf/pdfa/flavours/PDFFlavours.java (2)
64-67: LGTM! Correctly classifies WCAG_2_2_PDF_2_0 as PDF/UA-2 related.The addition of the
WCAG_2_2_PDF_2_0check properly extends PDF/UA-2 classification to include the new WCAG 2.2 PDF 2.0 flavours.
49-53: I need to examine the UA-2 classification method to fully verify the concern.Verify the review comment's accuracy regarding PDF/UA-1 classification.
The review comment contains a factual inaccuracy. PDF/UA-2 is based on PDF 2.0, but the codebase data shows that
WCAG_2_2usesPDFSpecification.ISO_32000_1_7(PDF 1.7), not PDF 2.0.The actual specification mappings are:
WCAG_2_1: PDF 2.0 → classified as PDF/UA-1 relatedWCAG_2_2: PDF 1.7 → classified as PDF/UA-1 related (consistent with PDF/UA-1 = PDF 1.7)WCAG_2_2_PDF_2_0: PDF 2.0 → classified as PDF/UA-2 related (consistent with PDF/UA-2 = PDF 2.0)The concern about
WCAG_2_1using PDF 2.0 but being grouped with UA1 may warrant verification, but the review mischaracterizesWCAG_2_2as using PDF 2.0. Additionally, WCAG 2.1 was published on 5 June 2018, which may explain why a PDF 2.0-based WCAG flavour could reasonably be associated with PDF/UA-1 if the classification is driven by WCAG version semantics rather than the underlying PDF specification version. To confirm whether the classification is intentional domain logic or requires correction, verify theisPDFUA2RelatedFlavourmethod and understand whyWCAG_2_1was mapped to UA1 despite using PDF 2.0.
MaximPlusov
force-pushed
the
wcag_2_0
branch
from
94334f4 to
8e9ffc4
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
core/src/main/java/org/verapdf/pdfa/flavours/PDFAFlavours.java (1)
83-83: LGTM! Constant serves its purpose correctly.The
PDF_2_0_PARTconstant successfully enables ID differentiation for PDF 2.0 variants in the getID logic.Optional: The name
PDF_2_0_PARTmight be slightly misleading since surrounding*_PARTconstants (e.g.,WCAG_2_2_PART,ISO_19005_1_PART) are integer part numbers, while this is a String postfix. Consider renaming toPDF_2_0_POSTFIXorPDF_2_0_SUFFIXfor clarity, though the current name is acceptable.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
core/src/main/java/org/verapdf/pdfa/flavours/PDFAFlavour.java(3 hunks)core/src/main/java/org/verapdf/pdfa/flavours/PDFAFlavours.java(1 hunks)core/src/main/java/org/verapdf/pdfa/flavours/PDFFlavours.java(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- core/src/main/java/org/verapdf/pdfa/flavours/PDFFlavours.java
🧰 Additional context used
🧬 Code graph analysis (1)
core/src/main/java/org/verapdf/pdfa/flavours/PDFAFlavour.java (1)
core/src/main/java/org/verapdf/pdfa/flavours/PDFAFlavours.java (1)
PDFAFlavours(34-117)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Codacy Static Code Analysis
🔇 Additional comments (3)
core/src/main/java/org/verapdf/pdfa/flavours/PDFAFlavour.java (3)
102-109: LGTM! Comments and enum declarations are now accurate.The updated comments correctly distinguish between WCAG 2.2 PDF 1.7 (lines 102-105) and PDF 2.0 (lines 106-109) variants, and the enum declarations properly reference the corresponding Specification constants.
134-135: LGTM! The postfix logic successfully resolves the duplicate ID issue.The conditional postfix appends
"2"to PDF 2.0 variant IDs, ensuring unique flavour identifiers:
- WCAG 2.2 PDF 1.7:
wcag2h,wcag2m- WCAG 2.2 PDF 2.0:
wcag2h2,wcag2m2This eliminates the FLAVOUR_LOOKUP collision and ensures
byFlavourId()andfromString()return the correct flavours.
222-227: LGTM! Specification mappings correctly reflect PDF baseline differences.The changes appropriately distinguish the two WCAG 2.2 variants:
WCAG_2_2now maps toISO_32000_1_7(PDF 1.7)WCAG_2_2_PDF_2_0maps toISO_32000_2_0(PDF 2.0)Note: Both specifications share the same
calculateID()result ("WCAG2.2") since they have identical part/subpart numbers. This appears intentional—they represent the same WCAG 2.2 standard applied to different PDF versions, with the key differentiation occurring at thePDFAFlavourenum level through the postfix mechanism.
1 participant
Summary by CodeRabbit
New Features
Enhancements
✏️ Tip: You can customize this high-level summary in your review settings.