feat(rules): model the Art. 8(1) declaration duty - #57
Conversation
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 18 |
| Duplication | 2 |
AI Reviewer: first review requested successfully. AI can make mistakes. Always validate suggestions.
TIP This summary will be updated as you push new changes.
There was a problem hiding this comment.
Pull Request Overview
The PR successfully models the core state machine for Art. 8(1) declaration duties (NotYetDue, Undetermined, NotCovered), but there is a significant implementation gap in the battery plugin's validation logic. While the documentation and regulatory notes correctly identify that a declaration must be anchored to both a reporting year and a manufacturing plant, the current implementation only checks for the presence of the reporting year. This renders the validation insufficient for full Art. 8(1) compliance.
Codacy analysis indicates the PR is up to standards, though it identified code duplication between the benchmark suite and the domain test support. Beyond the validation logic gap, the remaining findings are primarily nits regarding string literal formatting and missing structural type-checking for the new schema field. These issues should be resolved to ensure the reliability of the declaration duty reporting.
About this PR
- The
valid_battery()function inbenches/src/validation.rsis a duplicate ofsample_battery_data()incrates/dpp-domain/src/test_support.rs. This increases maintenance overhead as changes to the battery schema now require manual updates in multiple locations. Consider exposing the test support utilities via atest-supportfeature flag for reuse in benchmarks.
1 comment outside of the diff
benches/src/validation.rs
line 8⚪ LOW RISK
This test data setup is duplicated fromcrates/dpp-domain/src/test_support.rs. Refactoring this into a shared utility would eliminate the need for manual synchronization across crates.
Test suggestions
- art8_declaration_duty_for returns NotYetDue for dates preceding the category floor
- art8_declaration_duty_for returns Undetermined for dates on or after the floor while the delegated act is unadopted
- LMT batteries correctly trigger the 2033 duty floor instead of the 2028 floor
- Battery plugin emits a 'reporting_year_missing' warning when recycled content shares are present but the year is null
- Battery plugin remains silent if no recycled content shares are declared, regardless of the reporting year
- Verify that a battery can be in the Undetermined declaration state while still being in the NotYetBinding state for minimum shares (independent ladders)
TIP Improve review quality by adding custom instructions
TIP How was this review? Give us feedback
| if declared_any_share | ||
| && art8_category != Art8Category::NotCovered | ||
| && input.get("recycledContentReportingYear").is_none() |
There was a problem hiding this comment.
🟡 MEDIUM RISK
The Art. 8(1) declaration check is incomplete. According to the regulatory requirements cited in your documentation, shares must be anchored to both a reporting year AND a manufacturing plant to constitute a valid declaration. Currently, the check only verifies the year. Additionally, using .is_none() on the raw JSON input will fail to catch explicit null values which are permitted by the schema but invalid for the reporting duty.
Update the logic to ensure both recycledContentReportingYear and manufacturingPlace are present and non-null when shares are declared.
| pub const ART8_DECLARATION_FLOOR_2033: CalendarDate = CalendarDate::new(2033, 8, 18); | ||
|
|
||
| /// The empowerment whose entry into force can push the Art. 8(1) duty later. | ||
| pub const ART8_DECLARATION_EMPOWERMENT: &str = "EU 2023/1542 Art. 8(1), third subparagraph — recycled content calculation, verification and documentation format"; |
There was a problem hiding this comment.
⚪ LOW RISK
Nitpick: The constant ART8_DECLARATION_EMPOWERMENT contains an unintentional sequence of six spaces between 'calculation,' and 'verification'. Remove the excessive whitespace.
| warnings.push(PluginFinding::new( | ||
| "battery.recycled_content.reporting_year_missing", | ||
| "/recycledContentReportingYear", | ||
| "Recycled-content shares are declared without a reporting year. EU 2023/1542 Art. 8(1) requires them per battery model, per year and per manufacturing plant, so a share without its year is not the Art. 8(1) declaration.", |
There was a problem hiding this comment.
⚪ LOW RISK
Nitpick: This multi-line string literal includes the leading indentation as literal spaces, which will corrupt the formatting of the user-facing warning message. Use backslashes to escape the newline and indentation.
| "Recycled-content shares are declared without a reporting year. EU 2023/1542 Art. 8(1) requires them per battery model, per year and per manufacturing plant, so a share without its year is not the Art. 8(1) declaration.", | |
| "Recycled-content shares are declared without a reporting year. \ | |
| EU 2023/1542 Art. 8(1) requires them per battery model, per year \ | |
| and per manufacturing plant, so a share without its year is not \ | |
| the Art. 8(1) declaration.", |
Item 2 of the six left open by the citation audit. Art. 8(1) is a separate obligation from the Art. 8(2)/(3) minimums already modelled, with its own date ladder and its own granularity.
Two ladders, not one
A battery can owe the declaration without yet owing a minimum. Conflating them would report a shortfall against a duty that does not exist yet.
The date is conditional, and that changes the answer
Unlike 8(2)/(3), which state their dates outright, Art. 8(1) applies from the floor "or 24 months after the date of entry into force of the delegated act …, whichever is the latest" — and that act (due 18 Aug 2026) is unadopted.
So
art8_declaration_duty_forreturns three outcomes rather than a boolean:NotYetDuebelow the floor — certain, because the real date can only be the floor or later. This is the part that remains knowable while the act is pending.Undeterminedon or after the floor, naming the empowerment being waited on. Reporting "due" here would assert a date the regulation does not set.NotCoveredoutside scope.This is the
EFFECTIVE-DATES.md§4 principle applied to a concrete rule without building the full arithmetic: the floor is a lower bound, so a lower bound is what we answer with.A share without its year is not the declaration
Art. 8(1) requires the shares "for each battery model per year and per manufacturing plant". The model carried a bare percentage — an unattributed number that no reader can tie to a production run.
Schema v2.3.0 adds
recycledContentReportingYearto pair with the existingmanufacturingPlace, and the battery plugin flags shares declared without it. Same principle already applied to the carbon-footprint class (needs its ruleset) and state of health (needs its parameter set): a number without its provenance is not a declaration.Disclosure
Verified, no change needed: Annex XIII point 1(e) makes Art. 8(1) documentation public passport content, and the recycled-content fields correctly fall through to the public default.
Verification
just checkgreen: 818 tests, plugin suites included.just build-pluginsgreen.Eight new tests, including one asserting the two ladders stay independent — a battery placed on the market in 2029 is past the declaration floor but years short of the minimums.