## Problem ### `IsoDuration` states an averaging rule the code does not implement `src/Time/IsoDuration.php` says: > Month and year lengths fall back to averages (one month = 30.4375 days etc.) via > `\DateInterval` + epoch addition. The implementation adds the interval to the **epoch**, so month and year lengths are January-1970-anchored, not averaged: | duration | documented | actual | |---|---|---| | `P1M` | 30.4375 days | 31 days (January 1970) | | `P3M` | 91.3 days | 90 days (Jan + Feb + Mar 1970) | | `P2Y` | 730.5 days | 730 days (1970 and 1971 are both non-leap) | `P3M` and `P2Y` are both shipped defaults (`live_purge_after`, `file_purge_after`), so an operator reading the docblock to work out when a bucket is purged gets a different answer from the one the cron stage uses. The behaviour is self-consistent — one parser serves both sides — so what is wrong is the prose. ### Three copies of the parser, and the "single source of truth" is one of them `IsoDuration` calls itself the "Single source of truth for converting `P7D`, `PT0S`, `P3M`, etc. to microseconds". Two other places convert it themselves, with the same epoch-addition body: - `AuditTrailSettingsForm::getDurationSeconds()`, called three times in `validateForm()` on `new \DateInterval($raw)`; - `AuditTrailChainForm`, inline as `(new \DateTimeImmutable('@0'))->add(new \DateInterval($raw))`. Both re-implement the parse and the failure handling `IsoDuration::toMicroseconds()` and `tryToMicroseconds()` already own, and the settings form's copy repeats the same false averaging claim in its own docblock. The cron stage reaches the parser through a private one-line forward of its own, `getDurationMicroseconds()`, whose docblock says it exists so the call sites would not have to change. ### And two copies of the ordering rules, already worded differently The four rules that put a chain's stages in order — transient-purge before archive, archive before live-purge, live-purge before file-purge, file-purge before compaction — are implemented once per retention form. The copies have drifted where copies do. The same violation reads: - settings form: *live_purge_after must be greater than archive_after; otherwise rows would be live-purged before they are archived.* - chain form: *live_purge_after must be strictly greater than archive_after. Otherwise rows would be live-purged before they are archived.* The comparison is `>=` on both, so only one of those two sentences is accurate, and which one an operator reads is decided by which page they opened. The same holds for the other three rules. It is the divergence `EffectiveDelaysPreviewTrait` was extracted to stop, on the block directly above these fields. The copies differ in behaviour too: the global form checks its ordering only once all three required durations have parsed, so a typo in one postpones every complaint about the other two. ### A duration too long to measure is a TypeError, not a refusal `audit_trail.iso_duration` puts no ceiling on the year count, so `live_purge_after: P300000Y` passes the schema pattern and config can carry it. Measured in microseconds it does not fit in an integer: the multiplication returns a float, and under `strict_types` that is a TypeError out of a method typed `: int`. `tryToMicroseconds()` catches `\InvalidArgumentException` and nothing else, so it goes through the tolerant reader too, and its two callers are the auto-archive form and the effective-delays preview: a white screen on a route the operator asked for. The cron stage and both retention forms catch `\Throwable` and are not exposed. ### The effective-delays preview says "1 months" and "1 years" `EffectiveDelaysPreviewTrait::formatDuration()` builds every unit with a hard-coded plural: `t('@n hours')`, `'@n days'`, `'@n months'`, `'@n years'`. Drupal requires `formatPlural()` for a counted noun, and the singular case is reached by ordinary configurations, on the block whose whole purpose is telling an operator what their retention settings mean: - `archive_after: P30D` with `day` granularity: 31 days, `round(31 / 30.4375, 1)` = 1.0, rendered "~1 months"; - `file_purge_after: P1Y` with `day` granularity: 366 days, 1.0, rendered "~1 years"; - `transient_purge_after: PT23H` with `hour` granularity: 1.0 day, rendered "~1 days". ## Proposed resolution - Say what the parser does: the interval is measured from the epoch, so a month is the length of January and a year the length of 1970. Correct both docblocks. - Route the settings form and the chain form through `IsoDuration`, drop their copies and the cron stage's forward, and test that no second parse comes back. - One set of ordering rules for both forms, keeping the wording that states the comparison the code actually makes, and applying each rule as soon as both of its own stages have parsed. - Refuse a duration past what microseconds can hold, the way an unparseable one is refused, so every caller handles it with what it already has. - `formatPlural()` for all four units, with the real count where the value has one: `formatPlural()` picks the form from that number, and a stand-in count is a mistranslation everywhere English is not the language. AI-Generated: Yes (Claude Code was used to audit the module, to draft this issue and to write the fix and its tests. I reviewed all of it. Every test that pins a behaviour change was confirmed to fail against the unpatched code; the ones that pin the measured lengths pass either way, because what was wrong there was the prose.)