refactor(calc): select rulesets by governing-law date - #52
Conversation
Up to standards ✅🟢 Issues
|
| Metric | Results |
|---|---|
| Complexity | 91 |
| Duplication | 0 |
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 refactor successfully transitions the ruleset selection from a wall-clock approach to a fixed governing-law date (e.g., placedOnMarketDate). This ensures that compliance audits remain consistent over time, a critical requirement for EU regulatory alignment. Codacy analysis indicates the changes are up to standard with no new quality issues or complexity increases.
Key improvements include the introduction of the AssessmentClock structure, granular Assessability statuses, and corrected citations for Art. 8 recycled content. There are no critical flaws preventing a merge, though there is a minor risk of data loss in the Rust representation of performance classes (hardcoded A-E) compared to the now-dynamic JSON schema.
About this PR
- The
dpp_domain::CarbonFootprintClassRust type remains a hardcoded A-E enum despite the JSON schema being updated to allow arbitrary strings. This may lead to data loss during deserialization if future delegated acts define non-A-E labels. - The conditional application date arithmetic (calculating the max of a floor date and an entry-into-force offset) is currently deferred. While not immediately blocking as no delegated acts have entered into force, this remains a technical gap that will require implementation once the first acts are published.
Test suggestions
- AssessmentClock enforces that law_in_force_on is provided by the caller and not derived from the wall clock.
- Effectivity::Pending governs no dates and returns a RulesetUndetermined error naming the expected empowerment.
- Assessability enum correctly distinguishes between four distinct reasons for a ruleset being unavailable.
- Recycled content Phase 1 includes SLI batteries and industrial batteries > 2kWh starting precisely on 18 Aug 2031.
- Recycled content Phase 2 includes LMT batteries and previously covered categories starting on 18 Aug 2036.
- Battery plugin reports a missing or malformed placedOnMarketDate as a specific finding rather than assuming a default phase.
- resolve_repairability selects the correct ruleset version based on the law-in-force date.
TIP Improve review quality by adding custom instructions
TIP How was this review? Give us feedback
| // Percentages: verified correct per Annex X. | ||
| use crate::common::date::CalendarDate; | ||
|
|
||
| // ✅ COMPLIANCE-PIN: EU 2023/1542, Art. 8(2) and 8(3) (OJ L 191, 28.7.2023, p. 33) |
There was a problem hiding this comment.
🟡 MEDIUM RISK
Suggestion: Correcting the citation from Annex X to Art. 8(2)/(3) in the recycled content module is important for legal accuracy. Annex X relates to raw materials, while Art. 8 defines the actual thresholds.
| } | ||
|
|
||
| if let Some((ruleset_id, applies_from)) = not_yet { | ||
| Assessability::NotYetInForce { |
There was a problem hiding this comment.
🟡 MEDIUM RISK
Suggestion: The prioritization in resolve_repairability (favoring NotYetInForce over Undetermined and Expired) ensures that the most actionable regulatory status is returned, clearly distinguishing between known future obligations and unadopted legislative acts.
| /// let every determination silently re-derive the governing law from whichever | ||
| /// day the calculation happened to run. | ||
| #[derive(Debug, Clone, Copy, PartialEq, Eq, Serialize, Deserialize)] | ||
| pub struct AssessmentClock { |
There was a problem hiding this comment.
🟡 MEDIUM RISK
Suggestion: The introduction of AssessmentClock and the deliberate exclusion of a now() constructor is a robust design choice. By requiring the product's regulatory attachment date, the system ensures that audits of older products use the correct historical law.
| // Declining to guess is the whole point. Defaulting to "today" | ||
| // is what produces retroactively wrong findings once a phase | ||
| // begins, so say what is missing instead. | ||
| None => warnings.push(PluginFinding::new( |
There was a problem hiding this comment.
🟡 MEDIUM RISK
Suggestion: A missing or malformed placedOnMarketDate correctly prevents the plugin from asserting shortfalls, defaulting to a NotAssessed status. This design choice is critical for legal accuracy, as it prevents the system from defaulting to the current wall clock and making incorrect retroactive determinations. Ensure that downstream consumers are aware that NotAssessed is the expected state when this date is absent.
| /// determination keyed on a date must never run against one the declarer | ||
| /// did not actually state. | ||
| #[must_use] | ||
| pub fn parse_iso(s: &str) -> Option<Self> { |
There was a problem hiding this comment.
⚪ LOW RISK
Nitpick: The manual ISO 8601 parsing is functional and safe for no_std usage. It strictly requires the standard format with specific separators, which is appropriate for regulatory compliance where input formats must be rigid.
Selects the governing ruleset by the date the law attached to the product, not by the wall clock — plus the citation corrections that surfaced while verifying the dates against the Official Journal text of Regulation (EU) 2023/1542.
Primary text was read in full (OJ L 191, 28.7.2023, pp. 1-117). Research and provenance live in
dpp-docs/reference/regulatory/BATTERY-2023-1542-CITATION-MAP-2026-07.md.Citation corrections
Three wrong legal references on numerically correct rules:
annex_x_shortfalls_*renamed toart8_shortfalls_*.carbonFootprintClasshardcoded an A-E enum that does not exist in law. Art. 7(2) defines no labels and requires the Commission to "review the number of performance classes and the thresholds between them, every three years".REGULATORY.mdhad drifted further than the code it documents: both recycled-content phase dates were given as 1 January rather than 18 August, SLI was listed as excluded when Art. 8(2) names it, and LMT was placed in Phase 1 rather than Phase 2.The date model
Every application date in Art. 7 — and Art. 8(1), Art. 10(2)/(3) — is written as
max(fixed date, entry-into-force + N months). A fixedfromcannot express that.AssessmentClock { law_in_force_on, computed_at }is now the only way to reach a calculator. The wall-clockcalculate()overloads are deleted, not deprecated, and there is noAssessmentClock::now()— a convenience wrapper that reads the clock makes the wrong answer the shorter one to write, and the wrong answer here stays invisible until a phase boundary passes.Effectivity::{InForce, Pending}replacesEffectiveDateBound. The2100-01-01sentinel is gone: it asserted a calendar date for an act that does not exist.Pendinggoverns no date at all and yields a newCalcError::RulesetUndeterminednaming the instrument being waited on.Assessability<T>replacesOptiononresolve_*, separatingNotYetInForce/Undetermined/Expired/OutOfScope. None of those is non-compliance, andOptioncould not tell them apart.CalculationReceipt.assessed_as_ofrecords which law-date the calculation was performed against. Without it a receipt names the ruleset it cited but not whether that was the right one to cite.CalculatorStatusis now derived from each ruleset's ownEffectivityinstead of being hand-maintained beside it.placedOnMarketDateis added to battery schema v2.1.0 andBatteryData, and the battery plugin derives the Art. 8 phase from it. A missing or unparseable date produces its own finding and no determination.Breaking
Pre-1.0, landing in the unreleased 0.11.0 (latest published is 0.10.0, verified against the crates.io API).
dpp-enginepins 0.10.0 and references none of the changed symbols.annex_x_shortfalls_{2031,2036}->art8_shortfalls_{2031,2036}Ruleset::effective_dates()->effectivity();EffectiveDateBound->EffectivityRuleset::ensure_active_today()removedco2e::calculate/repairability::calculatetake anAssessmentClock;calculate_asofremovedCalculationReceipt::new/for_rulesettake anAssessmentClockresolve_repairabilityreturnsAssessability<T>currentSchemaVersion; v2.0.0 stays registered so existing passports remain verifiableNot in this PR
max(floor, entry-into-force + N months)arithmetic. No relevant act has entered into force, so it could only be validated against invented fixtures. Specified for whoever picks it up indocs/architecture/EFFECTIVE-DATES.md§4, including theConditionalvariant to add and the rule that entry-into-force is read from the OJ, never inferred.dpp_domain::CarbonFootprintClassis still anenum { A..E, Other }. The schema was fixed; the type was not.#[serde(other)]means an unrecognised label flattens toOtherand the original is lost on round-trip.degradation.rs, not built.Verification
just checkandjust build-pluginsboth green: 792 tests, 10 plugin suites.The gate itself had a hole —
plugins/sector-*are excluded from the workspace, sojust checknever ran their tests. Atest-pluginsrecipe is now wired intocheck; it immediately caught two real failures this branch had introduced that the old gate reported as green.Commits are split for review. History here is squash-merged, so they are not individually bisectable.