Close all ten findings from the business-logic audit - #12
Merged
Conversation
… has none A `fixed` holiday rule went straight to `days_from_civil`, which is Hinnant's algorithm and does not validate. A rule naming February 29 therefore produced March 1 in a common year - and the report NAMED that day as the holiday. April 31, a day that exists in no year, produced May 1 every year. The engine now answers `None` when the year's month is too short, exactly as `nth_weekday` already did for "the fifth Monday of February". A wrong day off is one error; a wrong day off carrying a holiday's name is two, and calendars are the one part of this tool outsiders are invited to write. The loader now bounds the day by the month's longest length in ANY year rather than a flat 1..=31, so February 29 still loads (it is a legitimate rule the engine resolves per year) while April 31 is refused as the typo it is - it would otherwise be a holiday silently absent forever. That bound comes from the core's own month lengths, so the calendar knowledge stays in one place. The loader comment claimed the engine resolved an impossible date per year. It did not. Fixed here too. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`year-2038` and `epoch-zero` each name one instant - the signed 32-bit `time_t` limit and epoch second zero. Both expressed it as `base.absolute`, which is wall-clock in the session zone (rule 2), so outside UTC they missed by exactly the zone offset: in UTC+02:00 the 2038 preset produced epoch 2_147_476_447 against a limit of 2_147_483_647, and reported NO significance marker at all. The tester ran "2038 boundary", never crossed it, and read green. Adds `base.absolute_utc` to the preset shape and `--base-utc` to calc, both read in UTC and re-expressed in the session zone before the first step runs. The conversion lives in the core (`Base::AbsoluteUtc`), so the GUI asks for it instead of computing its own - the two halves cannot drift apart on one moment. Adding a field does not bump the schema version, per the evolution table in docs/04 section 3. Refusals rather than silent picks: `absolute` together with `absolute_utc`, an explicit offset in `absolute_utc`, and a second `--base`/`--base-utc` flag. "Last one wins" there would move the moment by a zone offset without a word. The guard runs the presets at five biases and asserts the EPOCH SECOND, not the marker: `year_2038_boundary` means "at or past", so west of UTC a wrong moment lights the marker anyway and a marker-only check would pass for the wrong reason. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…'s weekend `observed` matches Saturday and Sunday by name - the variant names say so (`sun_to_mon`) - while `weekend` is a free list of days. A calendar pairing a Friday-Saturday weekend with a shift rule loaded cleanly and was then wrong twice over, in silence: a Friday holiday never shifted because the rule did not see it, and a Sunday holiday moved to Monday although Sunday is a working day there. Both surface as a wrong payment date with no message anywhere, in the one part of this tool outsiders are invited to write. The loader now refuses that combination and says what to use instead. Not generalised in the engine, because generalising needs new variant names: reusing `weekend_to_mon` for a weekend ending on Saturday would produce a Sunday while the name promises Monday, and misleading is worse than missing. That wants a real market to define it. The engine comment now states the assumption and points at the guard that enforces it, rather than leaving it as a promise nothing keeps. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…passing silently `h:mm tt` rendered `h:05 tt` - raw mask letters inside a string shaped like a formatted time. The vocabulary knew only `y M d H m s`, so the most common US time format was unreachable, and nothing said so: the reader's first guess is their own typo. The US is one of the two MVP markets and 12-hour with a designator is its rule (docs/02 section 7). Adds `hh`/`h` (midnight and noon are 12) and `tt`/`t`, and reports every letter run the vocabulary does not know - a line under the result in the human output, `custom_format_unknown` in JSON, a warning under the row in the GUI. The text still renders: this is a warning, not a refusal, because a partly-matching mask is still useful when the point is to mirror another app's output. Quoting comes with it, and not as a separate nicety. Adding a token changes what every unquoted mask containing that letter means - `date=` now loses its `d` to the day AND its `t` to the designator - so a warning with no way to answer it would have been half a fix. `'text'` is literal, `''` is an apostrophe, as in Java's SimpleDateFormat. Punctuation is never reported. A mask is made of it, and naming it would bury the finding. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`trial-last-day` computed `start + trial_length` at 23:59:59, so a 30-day trial started on 1 January pointed at 31 January. That is PAST the boundary for both usual implementations at once: an app checking `now - install > 30 days` sees 30 days and 14 hours, and one counting calendar days is on day 31. The preset whose question is "does it still work in the last moment" landed on a moment where the app already refuses, and the tester could not tell an app bug from a preset that misses. The install day is day one (owner's decision, 2026-09-08), so the last day is `start + length - 1` and the first day after is `start + length`. The pair still straddles one boundary exactly - they must move together or leave a gap. The rule is written into `explains` on both presets, not only into docs/05: that field is what the reader sees in the GUI and the CLI, and a document nobody opens while looking at a date does not tell them which boundary they are testing. The unit-test fixture that used to be a copy of a shipped preset is now named as a fixture. Pinning it to a real file's steps made a business-rule change look like a parser regression. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…o be noticed A month-folding shift that lands in a shorter month clamps the day: 31 January + 1 month is 28 February, 29 February + 1 year is 28 February. Every one of those is correct and documented, and every one is invisible in the result. A reader who sees a day change with no reason given cannot tell the rule from a defect - on a date they are about to act on. The engine now reports which step clamped and from what day, rather than each surface inferring it from a day that changed. The human render adds a line under the step, JSON carries `clamped_steps`, and the GUI shows a sentence under the result - it displays no intermediate steps at all, so a clamp was completely invisible there. This settles the fiscal-year finding (docs/05 3.4): a fiscal year starting on 29 February keeps ending on 27 February, because clamping is the product's rule everywhere else and the alternative needs a new shift mode in the step model shared with the GUI. The audit's own suggestion for it - swapping the step order - is wrong: it breaks a fiscal year starting on 1 March, which would then end on 28 February instead of 29. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Four small things, each one a place where the tool said something other than what it did. The numeric year check counted CHARACTERS, so `12/25/+999` was read as the year 999 while the error message beside it promises `N/N/YYYY`. Now four digits. A bare number is now read as an epoch, in seconds AND milliseconds, in the session zone. docs/02 8.1 has listed epoch among the recognised formats all along and the analyser refused it, which made pasting a number out of a log - the commonest thing a tester has in hand - an error. Both units are offered rather than one guessed from magnitude: `1000000000` is September 2001 in seconds and January 1970 in milliseconds, and both are things people paste. The "ambiguous" line now names WHICH ambiguity, because telling someone their epoch is a locale problem sends them to the wrong setting. The zone hour bound stays at 14 on both sides even though the real map stops at -12:00. This tool exists to put an app in a time it will not otherwise see, and refusing an offset because no country uses it drops coverage to buy nothing (untouchable rule 27). What was wrong was the message, which read as if 0..=14 were the map; it now says which part is real. A year the engine computes on but the moment field cannot hold (-0974-01-01, a real calculator result) now gets its own message instead of "use the form YYYY-MM-DD", which sent the reader to fix a shape that was already correct. Note on the first one: the audit's example was `-999`, which a dash sends to the ISO branch long before the check it was meant to expose. A test written on it passes with the bug in place - found by reverting, and the test now uses `+999`. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…rans Day switches `OffDays::rebuild` scans the year asked about and its two neighbours, because an observance can cross a year boundary in either direction: 1 January on a Saturday is observed on 31 December of the year before, and 31 December on a Sunday on 1 January of the year after. Nothing pinned that. The cache test next door compares a reused instance with a fresh one, and both would carry the same missing year - it cannot see this. It is the only place where the answer for one year depends on another year's data. Veterans Day has two switches, and the existing assertions sat years away from both (1965, 1975, 1978, 2026), so a window off by one passed all of them. Verified by extending only the October window by a year: every old assertion still passed and the new one caught it. The rest of the audit's suggested boundary tests turned out to already exist - Washington's Birthday and Memorial Day at 1970/1971, MLK at 1985/1986, Juneteenth at 2020/2021, Epiphany at 2010/2011, Christmas Eve at 2024/2025 are all pinned on the exact year. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
1 participant
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.
The date and calendar audit found ten issues (2 High, 3 Medium, 5 Low) and none had been fixed. All ten are closed here, in eight commits ordered by value for cost.
Every finding was originally measured against the running CLI rather than read out of the code, and every fix was verified the same way, plus a revert probe on a full test run.
The two High findings
A calendar could name a holiday on a date that has none. A
fixedrule went straight to Hinnant's algorithm, which does not validate, so a rule naming February 29 produced March 1 in a common year - and the report attached the holiday's NAME to it. April 31, a day that exists in no year, produced May 1 every year. The engine now answers "not this year", exactly asnth_weekdayalready did, and the loader refuses a day impossible in every year. The loader comment claiming the engine already resolved this is fixed too.Two presets hit their target only in UTC.
year-2038andepoch-zeroeach name one instant, and both expressed it as session-zone wall-clock. In UTC+02:00 the 2038 preset produced epoch 2,147,476,447 against a limit of 2,147,483,647, with no significance marker at all - the test ran green without touching the boundary. Both now use a newbase.absolute_utc, with the conversion in the core (--base-utcfor callers) so the GUI does not get its own instant arithmetic.Medium and Low
hh/handtt/t, reports letter runs it does not recognise instead of passing them through as if they were output, and gained'quoting'- which had to come with the new token, since addingtchanges what every unquoted mask containing that letter means.trial-last-dayno longer lands past the boundary it exists to sit inside. The rule is written intoexplains, which is what the reader actually sees.Two suggestions from the audit that did not survive checking
Its fix for the fiscal-year finding - swapping the step order - breaks a fiscal year starting on 1 March, which would then end on 28 February instead of 29.
Its example for the year-digit finding,
12/25/-999, contains a dash and is therefore dispatched to the ISO branch before reaching the check it was meant to expose. A test written on that example passes with the bug still in place. The real case is12/25/+999, found only because the revert probe came back green.The zone-range narrowing is declined on purpose: this tool exists to put an app in a time it will not otherwise see, and refusing an offset because no country uses it drops coverage to buy nothing.
Contracts
Neither schema version moves. Only fields were added (
absolute_utc,custom_format_unknown,clamped_steps), which the evolution table treats as non-breaking. A user of the released build who swaps in only the new preset files gets a loud refusal, never a silently shifted moment.Gates
12/12 local gates, Rust 419 -> 432 tests, C# 365 -> 373, target harness 146 PASS / 0 FAIL on x64.
🤖 Generated with Claude Code