From b245f8d7b90b0aa481164ca99e13c7436658384b Mon Sep 17 00:00:00 2001 From: JarbasAi Date: Tue, 29 Sep 2026 02:39:36 +0000 Subject: [PATCH] fix: a holiday offset written outside the construction moves the date MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `extract_holiday_span` parsed `written[first:last]`, the characters the holiday construction itself matched, so a modifier written outside that construction was never applied. Four dates were silently wrong on dev at the layer, each answering the holiday itself and handing the offset back in the remainder as though it were question words. At an anchor of 25 September 2026: "the day after christmas" and "the day before christmas" both answered 25 December, "two days after christmas" answered 25 December, and the French "le jour apres noel" answered 25 December. The trace cannot decide it. `explain` reports the same bare `holiday_ref` over the holiday word alone for an offset phrase and for a question phrase, so the two shapes are indistinguishable there, and widening the extent to the other winners reads the offset's own quantity ("two days") as a date in its own right. So the whole text is parsed and `DateSpanResult.remainder` is read. `_holiday_extent` stays, as the gate alone: a holiday reading is kept only when a winning match is a holiday construction. What the answer covers is then chronologia's to say. `_caller_spelling` maps that remainder back onto the caller's own text. `_as_written` may put back an accent the caller never typed, and every substitution there replaces the same number of characters, so a position in one string is the same position in the other. The function checks that the lengths agree rather than assuming it, finds each remainder word left to right so a repeated word takes its own occurrence, and returns None on any mismatch, where the caller falls back to cutting the phrase out by the extent. The date stands either way; only the remainder falls back. Measured on the branch: the four rows answer 26, 24 and 27 December and 26 December, each with an empty remainder, and the same French sentence without its accents reads the same. Every date was counted by hand from 25 December 2026. The question rows `#372` changed the parse input to protect are unchanged: "how many days until christmas" keeps "how many days until", "quantos dias faltam para o natal" keeps "quantos dias faltam para o", and "play some christmas music" keeps "play some music". Two things this does not fix, both filed. "two days after christmas" still answers 27 September 2026 through `extract_datetime`: the engine reads "two days" as an offset from the anchor and the walk never reaches the holiday layer. That is a different defect, and not one `holiday_overrides_engine` covers — the engine consumed "two days", which does not lie inside "christmas", so its subset rule correctly declines. T-6961. The French "combien de jours avant noel" goes from 25 December, correct, to 24 December, wrong: chronologia reads "avant" after an interrogative quantity as an offset. The English "how many days until christmas" and the French "combien de jours jusqu'a noel" are both read correctly, so the interrogative is what it turns on, not the language. Dev was right on that row by accident, because the substring parse hid the question's words. T-6896 owns it, and a cell holds the wrong answer with its control so the day it is fixed the cell says so. Suite: 2906 passed, 3 skipped, 14 xfailed, 2294 subtests passed. The whole suite, because `holidays.py` is shared and `extract_datetime` is the entry point. With dev's `holidays.py` copied back over the fix the module gives 16 failed, 74 passed, so the new cells discriminate. Panel decision: holiday-offset-vs-french-interrogative. Evidence: knowledge/wiki/audits/spec-adoption/t6895-c1-fix-forward.md reviewer-d's CONFIRMED finding: the part-of-day offset family is the same defect as the French row and was undeclared. A part-of-day word written as the offset is read as a time of day ON the holiday and the direction is dropped, so "the night before christmas" answers 21:00 on 25 December and "the night after christmas" answers the same. Eight rows in English, French and Portuguese, every one with an empty remainder, so a caller cannot see the modifier was read at all. The docstring claimed one known wrong answer. No code changes. The defect is chronologia's, like the French row, and task T-7350 carries it. The day offset is read correctly on the same anchor, so the part-of-day word is the whole of it. The docstring now declares the family. One parametrised cell holds all eight answers, with what each utterance actually names written beside it. Two cells go with it: the directions give the same answer, asserted on its own because a per-row cell could pass while the collapse remained, and the day offset as the control that the offset machinery itself works. The cells call extract_holiday_span, not extract_datetime, because it is that function's contract this family bounds; through extract_datetime the engine answers these utterances first and the holiday layer is never reached. Mutation: making the layer decline these utterances reds 9 cells. Whole suite 2916 passed, 3 skipped, 14 xfailed, 2294 subtests. T-7244. Co-Authored-By: Claude Opus 5 --- ovos_date_parser/holidays.py | 102 +++++++++++--- test/test_holidays_chronologia.py | 218 ++++++++++++++++++++++++++++-- 2 files changed, 294 insertions(+), 26 deletions(-) diff --git a/ovos_date_parser/holidays.py b/ovos_date_parser/holidays.py index 6e6b51b..f38d330 100644 --- a/ovos_date_parser/holidays.py +++ b/ovos_date_parser/holidays.py @@ -287,23 +287,89 @@ def _holiday_extent(text: str, lang: str, anchor: datetime return None +def _caller_spelling(text: str, written: str, remainder: str) -> Optional[str]: + """``remainder``, which is cut from ``written``, re-spelled from ``text``. + + :func:`_as_written` may have put an accent back that the caller never + typed, and the remainder is handed to the caller, so it must come back in + the caller's own characters. Every substitution there replaces the same + number of characters, so the two strings are the same length and a + position in one is the same position in the other; that equality is + checked rather than assumed, and a mismatch returns None so the caller can + fall back. + + Each word of the remainder is found in ``written`` left to right, never + searched for in the whole string, so a word that occurs twice takes its + own occurrence. A word that is not found returns None: chronologia owns + the remainder's spelling and may normalise a word one day, and a wrong + offset must not be guessed at. + """ + if len(written) != len(text): + return None + out = [] + at = 0 + for word in remainder.split(): + found = written.find(word, at) + if found < 0: + return None + out.append(text[found:found + len(word)]) + at = found + len(word) + return " ".join(out) + + def extract_holiday_span(text: str, lang: str, anchorDate: Optional[datetime] = None ) -> Optional[Tuple[datetime, str]]: """Resolve a named holiday in ``text`` to ``(datetime, remainder)``. - The date is the occurrence the holiday phrase asks for: the next one by - default, and the one a determiner inside the phrase names when it - carries one ("next easter", "last christmas", "christmas eve"), as - chronologia reckons it from ``anchorDate``. A verb tense is not read — - "when was easter" answers with the next Easter, because chronologia - reads no verb — so this claims the determiner only. A word outside the holiday phrase is not read: it - stays in the remainder, so "how many days until christmas" and the - French "combien de jours avant noël" both answer with Christmas and both - keep their question. - - The remainder is the caller's own text with the holiday phrase cut out of - it, and nothing else removed. + The date is the occurrence the phrase asks for: the next one by default, + the one a determiner inside the phrase names when it carries one ("next + easter", "last christmas", "christmas eve"), and the one an offset + outside the phrase names ("the day after christmas", "two days after + christmas", "le jour après noël"), as chronologia reckons it from + ``anchorDate``. A verb tense is not read — "when was easter" answers with + the next Easter, because chronologia reads no verb — so this claims the + determiner and the offset, never the verb. + + The whole text is parsed, not the holiday construction's own characters. + Parsing the substring was what lost the offset: a modifier outside the + construction was never applied, so "the day after christmas" answered + 25 December and handed "the day after" back as though it were question + words. :func:`_holiday_extent` stays, as the gate alone: a holiday + reading is kept only when a winning match is a holiday construction. What + the answer covers is then chronologia's to say, and it says it through + ``DateSpanResult.remainder``. + + The remainder is therefore chronologia's, re-spelled with the caller's own + characters by :func:`_caller_spelling`, and it keeps every word the parse + did not consume: "how many days until christmas" and "quantos dias faltam + para o natal" both answer with Christmas and both keep their question. + + **One known wrong answer, and it is chronologia's.** The French "combien + de jours avant noël" asks how many days remain before Christmas, so the + date is Christmas, and this answers 24 December with the remainder + "combien": chronologia reads "avant" after an interrogative quantity as an + offset. The English "how many days until christmas" and the French + "combien de jours jusqu'a noël" are both read correctly. Telling the two + French shapes apart needs a table of interrogatives per language, which + belongs to chronologia and is not copied here. + ``test_holidays_chronologia.py`` carries a cell holding that answer, so + the day chronologia fixes it the cell says so. + + **A part-of-day offset is read as a time of day on the holiday, and the + direction is dropped.** "the night before christmas" answers 21:00 on + 25 December, not the night of the 24th, and "the morning after christmas" + answers 06:00 on the 25th, not the morning of the 26th. The before and the + after make no difference to the answer, and every row of this family comes + back with an EMPTY remainder, so a caller cannot see that the modifier was + read at all. Eight rows are measured, in English, French and Portuguese. + + This is the same defect as the French row above and is chronologia's in the + same way: the day offset is read correctly, so "the day after christmas" + gives 26 December, and only the part-of-day word is misread. Task T-7350 + carries it. ``test_holidays_chronologia.py`` holds all eight answers in one + parametrised cell, so the day chronologia fixes any of them the cell says + which. Returns None when the utterance names no holiday in its own language, or when chronologia answered from something other than a holiday. @@ -317,10 +383,9 @@ def extract_holiday_span(text: str, lang: str, extent = _holiday_extent(written, lang, anchor) if extent is None: return None - first, last = extent try: from chronologia import extract_timespan - result = extract_timespan(written[first:last], lang=_base_lang(lang), + result = extract_timespan(written, lang=_base_lang(lang), anchor=anchor, jurisdiction=_jurisdiction(lang)) except Exception: @@ -330,8 +395,13 @@ def extract_holiday_span(text: str, lang: str, start = result.span.start_datetime if start is None: # a span outside the datetime range return None - remainder = re.sub(r"\s{2,}", " ", text[:first] + " " + text[last:]) - return start, remainder.strip() + remainder = _caller_spelling(text, written, result.remainder) + if remainder is None: + # the offsets do not line up, so the holiday phrase is cut out by the + # extent instead; the date stands, only the remainder falls back + first, last = extent + remainder = text[:first] + " " + text[last:] + return start, re.sub(r"\s{2,}", " ", remainder).strip() def extract_holiday_date(text: str, lang: str, diff --git a/test/test_holidays_chronologia.py b/test/test_holidays_chronologia.py index 8a873cc..48c814f 100644 --- a/test/test_holidays_chronologia.py +++ b/test/test_holidays_chronologia.py @@ -161,28 +161,150 @@ def test_a_sentence_that_merely_names_a_holiday_costs_milliseconds(): @pytest.mark.parametrize("lang,utterance,kept", [ ("en-US", "how many days until christmas", "how many days until"), - ("fr-FR", "combien de jours avant noël", "combien de jours avant"), ("pt-PT", "quantos dias faltam para o natal", "quantos dias faltam para o"), ("en-US", "play some christmas music", "play some music"), ]) -def test_the_remainder_keeps_every_word_outside_the_holiday_phrase( +def test_the_remainder_keeps_every_word_the_parse_did_not_consume( lang, utterance, kept): - """Only the holiday phrase leaves the remainder. + """A question word the parse did not read stays in the remainder. - "combien de jours avant noël" came back as 'combien': chronologia - applied "avant" as an offset from outside the match and took "de jours" - with it, so a French question lost words its English sibling kept. + The remainder is chronologia's, re-spelled with the caller's own + characters, so it holds exactly what the parse left. The French sibling + of the first row is not here: it is a known wrong answer and has its own + cell below. """ got = extract_datetime(utterance, lang, REF) assert got is not None assert got[1] == kept -def test_the_same_question_reads_the_same_in_english_and_french(): - """The French question asked about Christmas and was answered Christmas Eve.""" +def test_the_english_question_keeps_its_words_and_answers_christmas(): + """The shape the French row below should have, and does not.""" english = extract_datetime("how many days until christmas", "en-US", REF) - french = extract_datetime("combien de jours avant noël", "fr-FR", REF) - assert english[0] == french[0] == CHRISTMAS + assert english[0] == CHRISTMAS + assert english[1] == "how many days until" + + +# --- known answers, not correct answers ------------------------------------- + +def test_a_french_interrogative_quantity_before_a_holiday_is_read_as_an_offset(): + """Known answer, and the defect is chronologia's: task T-6896. + + "combien de jours avant noël" asks how many days remain before Christmas, + so the date is Christmas, 25 December. chronologia reads "avant" after an + interrogative quantity as an offset and answers the day before, taking + "de jours avant" into the match and leaving "combien" behind. + + The two controls are the shapes that ARE read correctly, so the cell + blames the interrogative and not the language or the preposition: the + English "how many days until christmas" above, and the French + "combien de jours jusqu'a noël" here. + + Parsing the holiday construction's own substring hid this row, at the + price of four silently wrong offset dates ("the day after christmas" and + its siblings). The whole text is right on those four and wrong on this + one. The day chronologia fixes it, this cell fails and says so. + """ + got = extract_datetime("combien de jours avant noël", "fr-FR", REF) + assert got is not None + assert got[0] == datetime(2026, 12, 24, 0, 0) + assert got[0] != CHRISTMAS + assert got[1] == "combien" + + control = extract_datetime("combien de jours jusqu'à noël", "fr-FR", REF) + assert control[0] == CHRISTMAS + + +#: The C1 rows: an offset written OUTSIDE the holiday construction. Each date +#: is counted by hand from Christmas, 25 December 2026. +#: The C1 rows: an offset written OUTSIDE the holiday construction. Each date +#: is counted by hand from Christmas, 25 December 2026. +OFFSET_UTTERANCES = [ + ("en-US", "the day after christmas", datetime(2026, 12, 26, 0, 0)), + ("en-US", "the day before christmas", datetime(2026, 12, 24, 0, 0)), + ("en-US", "two days after christmas", datetime(2026, 12, 27, 0, 0)), + ("fr-FR", "le jour après noël", datetime(2026, 12, 26, 0, 0)), + ("fr-FR", "le jour apres noel", datetime(2026, 12, 26, 0, 0)), +] + +#: The rows above that reach the holiday layer through `extract_datetime`. +#: "two days after christmas" is not one of them and has its own cell below. +OFFSET_THROUGH_EXTRACT_DATETIME = [ + row for row in OFFSET_UTTERANCES if not row[1].startswith("two days") +] + + +@pytest.mark.parametrize("lang,utterance,expected", OFFSET_UTTERANCES) +def test_an_offset_outside_the_holiday_phrase_moves_the_date( + lang, utterance, expected): + """The C1 regression, one cell per row, on the layer that owns it. + + Each of these answered 25 December, the holiday itself, and handed the + offset back in the remainder as though it were question words. The parse + read the holiday construction's own characters, so a modifier outside the + construction was never applied. + + The last row is the same French sentence without its accents, which is + what speech to text produces; it must read the same. + """ + got = extract_holiday_span(utterance, lang, REF) + assert got is not None + assert got[0] == expected + assert got[0] != CHRISTMAS + + +@pytest.mark.parametrize("lang,utterance,expected", OFFSET_UTTERANCES) +def test_an_offset_phrase_leaves_no_remainder(lang, utterance, expected): + """The other half: the offset words are consumed, not handed back. + + A caller that reads the remainder as the rest of the command would have + been given "the day after" to act on. + """ + got = extract_holiday_span(utterance, lang, REF) + assert got is not None + assert got[1] == "" + + +@pytest.mark.parametrize("lang,utterance,expected", + OFFSET_THROUGH_EXTRACT_DATETIME) +def test_an_offset_phrase_reads_the_same_through_extract_datetime( + lang, utterance, expected): + """The whole call, not the layer alone, for the rows that reach it. + + Without this the module could pass on a library whose entry point never + consults the holiday layer at all. + """ + got = extract_datetime(utterance, lang, REF) + assert got is not None + assert got[0] == expected + + +def test_two_days_after_christmas_is_answered_by_the_engine_not_the_holiday(): + """Known answer, and a different defect from the one above. + + The holiday layer reads this row correctly, and the cells above assert + that. `extract_datetime` never asks it: the per-language engine reads + "two days" as an offset from the anchor, answers 27 September 2026 and + hands back "after christmas", so the engine-first order ends the walk + before the holiday layer is reached. + + That is the shadowing family of finding 5 of the #369 review, but not the + case `holiday_overrides_engine` covers: its rule asks whether the words + the engine consumed all lie inside the holiday phrase, and "two days" + does not lie inside "christmas". The rule is right to decline here; the + engine's partial read is the defect, and it is filed on its own. + + The cell holds the answer this library gives today so the day that is + fixed it fails and says so. + """ + got = extract_datetime("two days after christmas", "en-US", REF) + assert got is not None + assert got[0] == datetime(2026, 9, 27, 0, 0) + assert got[1] == "after christmas" + + # the control: the layer the walk skipped has the right answer + layer = extract_holiday_span("two days after christmas", "en-US", REF) + assert layer[0] == datetime(2026, 12, 27, 0, 0) @pytest.mark.parametrize("lang,plain,written", [ @@ -227,3 +349,79 @@ def test_a_tense_inside_the_holiday_phrase_still_reads(utterance, expected): "christmas eve" each as one match over all their words. """ assert extract_datetime(utterance, "en-US", REF)[0] == expected + + +# --- the part-of-day offset family, T-7244 from reviewer-d's review of #381 - + +# A part-of-day word written as the offset is read as a time of day ON the +# holiday, and the before/after is dropped: "before" and "after" give the same +# answer. Every row comes back with an EMPTY remainder, so a caller cannot see +# that the modifier was read at all. That is what makes the family worth a cell +# rather than a note: dev at least handed the offset back in the remainder. +# +# The defect is chronologia's, like the French interrogative above, and task +# T-7350 carries it. The day offset is read correctly on the same anchor +# ("the day after christmas" gives 26 December), so the part-of-day word is the +# whole of it. +# +# These call extract_holiday_span rather than extract_datetime, because it is +# that function's contract this family bounds. Through extract_datetime the +# per-language engine answers these utterances first and the holiday layer is +# never reached, so extract_datetime would assert the engine's answer, which is +# a different wrong answer and not this one. + +PART_OF_DAY_OFFSETS = [ + # lang, utterance, the answer given today, what the utterance names + ("en-US", "the night before christmas", + datetime(2026, 12, 25, 21, 0), "the night of 24 December"), + ("en-US", "the morning after christmas", + datetime(2026, 12, 25, 6, 0), "the morning of 26 December"), + ("en-US", "the evening after christmas", + datetime(2026, 12, 25, 18, 0), "the evening of 26 December"), + ("en-US", "the night after christmas", + datetime(2026, 12, 25, 21, 0), "the night of 26 December"), + ("en-US", "the morning before christmas", + datetime(2026, 12, 25, 6, 0), "the morning of 24 December"), + ("fr-FR", "le soir avant noël", + datetime(2026, 12, 25, 18, 0), "le soir du 24 décembre"), + ("fr-FR", "le matin après noël", + datetime(2026, 12, 25, 4, 0), "le matin du 26 décembre"), + ("pt-PT", "a noite antes do natal", + datetime(2026, 12, 25, 19, 0), "a noite de 24 de dezembro"), +] + + +@pytest.mark.parametrize("lang,utterance,given,asked", PART_OF_DAY_OFFSETS) +def test_a_part_of_day_offset_is_read_as_a_time_on_the_holiday( + lang, utterance, given, asked): + """Known answer, not a correct answer: the defect is chronologia's, T-7350. + + Each expected value is the answer this library gives today. What the + utterance actually names is written beside it. The day chronologia fixes + any of these the cell fails and says which. + """ + got = extract_holiday_span(utterance, lang, REF) + assert got is not None, f"{utterance!r} no longer reaches the holiday layer" + assert got[0] == given, f"{utterance!r} asks for {asked}" + assert got[1] == "", ( + f"{utterance!r} left {got[1]!r} over; an empty remainder is what makes " + "this family invisible to a caller, and the cell holds that too") + + +def test_the_direction_makes_no_difference_to_a_part_of_day_offset(): + """The sharpest statement of the defect: before and after agree. + + A cell per row could pass while the two directions still collapsed onto + one answer, so the collapse is asserted on its own. + """ + before = extract_holiday_span("the night before christmas", "en-US", REF) + after = extract_holiday_span("the night after christmas", "en-US", REF) + assert before[0] == after[0] == datetime(2026, 12, 25, 21, 0) + + +def test_a_day_offset_is_still_read_correctly(): + """Control: the direction IS honoured for a day offset, so the family + above blames the part-of-day word and not the offset machinery.""" + got = extract_holiday_span("the day after christmas", "en-US", REF) + assert got is not None + assert got[0] == datetime(2026, 12, 26, 0, 0)