fix: a holiday named after a weekday answers with the holiday

ovos_date_parser/__init__.py

@@ -29,6 +29,7 @@
 from ovos_date_parser.scoped_en import extract_scoped_date_en, SCOPED_VOCAB_EN
 from ovos_date_parser.holidays import (extract_holiday_date,
                                        extract_holiday_span,
+                                       holiday_overrides_engine,
                                        holiday_surfaces)
 from ovos_date_parser.eras_en import extract_era_date_en, ERA_PATTERNS_EN
 from ovos_date_parser.eras_pt import extract_era_date_pt, ERA_PATTERNS_PT
@@ -412,15 +413,28 @@ def extract_datetime(
     reads no date, :func:`ovos_date_parser.holidays.extract_holiday_span`
     asks chronologia, and its answer is taken only when chronologia says a
     holiday construction is what matched. An utterance the engine already
-    reads keeps the engine's answer, so nothing that worked before changes.
+    reads keeps the engine's answer, with one exception: a holiday whose own
+    name is written with calendar vocabulary ("good friday", "palm sunday")
+    is read by the engine as that weekday, so when the engine answers the
+    holiday phrase on its own with the same date it gave the whole text
+    (:func:`~ovos_date_parser.holidays.holiday_overrides_engine`) the holiday
+    layer answers instead. A time of day, an ordinal or any other word beside
+    the holiday name does not stop this, because it does not move the date.
+    A date the engine read from a calendar word the phrase does not cover is
+    a different date, and stays the engine's.
     """
     found = _extract_datetime_engine(text, lang, anchorDate=anchorDate,
                                      default_time=default_time)
-    if found is not None:
+    if found is not None and not holiday_overrides_engine(
+            text, lang, found[0], found[1], anchorDate=anchorDate):
         return found
     holiday = extract_holiday_span(text, lang, anchorDate=anchorDate)
     if holiday is None:
-        return None
+        # one return for both ways of arriving here: the engine read nothing
+        # and ``found`` is None, which is the old contract, or the override
+        # sent an engine answer to a holiday layer that then declined, and
+        # the engine's answer stands.
+        return found
     moment, remainder = holiday
     if default_time is not None:
         moment = moment.replace(hour=default_time.hour,

ovos_date_parser/holidays.py

@@ -334,6 +334,103 @@ def extract_holiday_span(text: str, lang: str,
     return start, remainder.strip()
 
 
+def holiday_overrides_engine(text: str, lang: str, engine_date: datetime,
+                            engine_remainder: str = "",
+                            anchorDate: Optional[datetime] = None) -> bool:
+    """Whether a language engine read its date out of the holiday phrase.
+
+    A holiday name may be written with calendar vocabulary of its own
+    language: "good friday", "palm sunday", "may day". A per-language engine
+    reads the weekday inside such a name and answers with the coming Friday,
+    and the holiday layer, asked only when the engine found nothing, is never
+    reached. The engine order is right everywhere else, so the question asked
+    here is narrow: did the engine take its date from the holiday phrase
+    itself?
+
+    Args:
+        text: the text the engine was given.
+        lang: the BCP-47 code the engine was given.
+        engine_date: the datetime the engine answered with. Its date is the
+            value compared; its time is not read.
+        engine_remainder: what the engine left over. Read only by the cheap
+            guard below, which declines without a second engine call when the
+            engine consumed no word of the holiday phrase.
+        anchorDate: the date relative dating is reckoned from.
+
+    The question is answered by asking the engine the holiday phrase on its
+    own. The override fires when that answer's date equals ``engine_date``,
+    the date the engine gave the whole text. For "good friday" the slice
+    "good friday" gives the coming Friday and so does the whole text, so the
+    engine read its date out of the phrase. For "play christmas music on
+    friday" the slice is "christmas", which the engine cannot read at all, so
+    the Friday came from elsewhere and stays the engine's. For "good friday
+    and next friday" the slice gives the coming Friday and the whole text
+    gives the one after, so the dates differ and the engine keeps its answer.
+
+    Only the date is compared, never the time. A time of day beside the
+    holiday name does not move the date, so "good friday at 9am" and
+    "remind me on good friday at 9am" override exactly as the bare name does.
+    That is this function's reason for existing in this shape: the first cut
+    compared the words the engine consumed against the words of the phrase
+    and declined whenever the engine consumed anything else, which left half
+    the holiday's name in the remainder on the commonest spoken form of all
+    eight of these holidays.
+
+    The holiday layer answers with a date, so the engine's time of day is not
+    carried onto it. "good friday at 9am" gives Good Friday at midnight and
+    leaves "at 9am" in the remainder.
+
+    A date word that moves the date keeps the engine's answer, which is the
+    right outcome for "good friday and next friday" and the wrong one for
+    "the friday before good friday": the slice and the whole text both give
+    the coming Friday there, so the override fires and the answer is Good
+    Friday rather than the Friday before it. "monday after easter monday"
+    is the same shape. Both were already wrong before this function existed,
+    where the engine gave the coming weekday, so this is a bound on the fix
+    and not a regression. To decide it properly, compare character offsets
+    against the extent ``(first, last)`` computed below.
+    ``test_holidays_chronologia.py`` carries a cell for each known answer, so
+    the day the extent becomes positional the cell says what changed.
+
+    The engine is asked twice for a text that names a holiday of its
+    language, once by the caller and once here, so such a text costs more
+    than one that names none. A text that names no holiday pays nothing: the
+    table lookup below answers first.
+
+    A text that names no holiday of its language never reaches chronologia:
+    :func:`_names_a_holiday` is a table lookup and answers first.
+    """
+    if not text:
+        return False
+    if not _names_a_holiday(text, lang):
+        return False
+    anchor = anchorDate or datetime.now()
+    written = _as_written(text, lang)
+    extent = _holiday_extent(written, lang, anchor)
+    if extent is None:
+        return False
+    first, last = extent
+    phrase_words = _word_set(written[first:last])
+    consumed = _word_set(text) - _word_set(engine_remainder)
+    if not (consumed & phrase_words):
+        # The engine read its date from words the phrase does not cover, so
+        # the slice below cannot give the same date except by coincidence.
+        # Declining here is what keeps a text that merely names a holiday
+        # ("play christmas music on friday") to one engine call.
+        return False
+    # imported here, not at module scope: ovos_date_parser/__init__.py imports
+    # this module, so a top-level import would be a cycle.
+    from ovos_date_parser import _extract_datetime_engine
+    inner = _extract_datetime_engine(written[first:last], lang,
+                                     anchorDate=anchor)
+    return inner is not None and inner[0].date() == engine_date.date()
+
+
+def _word_set(text: str) -> FrozenSet[str]:
+    """The folded words of ``text``, for comparing one extent with another."""
+    return frozenset(_fold(word) for word in re.findall(r"\w+", text or ""))
+
+
 def extract_holiday_date(text: str, lang: str,
                          ref_date: Optional[date] = None
                          ) -> Optional[Tuple[date, str]]:

test/test_holidays_chronologia.py

@@ -8,7 +8,7 @@
 The anchor is fixed at 25 September 2026, so "the next one" is a stated date
 rather than whatever today makes it.
 """
-from datetime import date, datetime
+from datetime import date, datetime, time
 
 import pytest
 
@@ -227,3 +227,199 @@ 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
+
+
+# --- finding 5 of the #369 review: a holiday named after a weekday ----------
+
+# `extract_datetime` runs the per-language engine first. A holiday whose own
+# name carries weekday vocabulary — "good friday", "palm sunday" — therefore
+# answered with the coming weekday and never reached the holiday layer, which
+# knew the right date all along. Eight of the twelve weekday- and month-named
+# English holidays answered that way.
+#
+# Every date below is reckoned independently of the parser. Western Easter 2027
+# is 28 March by the Gregorian computus, and the movable feasts are counted from
+# it: Palm Sunday is Easter minus 7 (21 March), Maundy Thursday minus 3
+# (25 March), Good Friday minus 2 (26 March), Holy Saturday minus 1 (27 March),
+# Easter Monday plus 1 (29 March), Whit Monday plus 50 (17 May). Shrove Tuesday
+# is Easter minus 47 (9 February) and Ash Wednesday minus 46 (10 February).
+
+WEEKDAY_NAMED_HOLIDAYS = [
+    ("palm sunday", date(2027, 3, 21)),
+    ("maundy thursday", date(2027, 3, 25)),
+    ("good friday", date(2027, 3, 26)),
+    ("holy saturday", date(2027, 3, 27)),
+    ("easter monday", date(2027, 3, 29)),
+    ("whit monday", date(2027, 5, 17)),
+    ("shrove tuesday", date(2027, 2, 9)),
+    ("ash wednesday", date(2027, 2, 10)),
+]
+
+
+@pytest.mark.parametrize("utterance,expected", WEEKDAY_NAMED_HOLIDAYS)
+def test_a_weekday_named_holiday_answers_with_the_holiday(utterance, expected):
+    """The holiday layer's date wins when the engine read the holiday's own
+    words as a weekday."""
+    got = extract_datetime(utterance, "en-US", REF)
+    assert got is not None
+    assert got[0].date() == expected
+
+
+@pytest.mark.parametrize("utterance,expected", WEEKDAY_NAMED_HOLIDAYS)
+def test_a_weekday_named_holiday_keeps_no_half_of_its_name(utterance, expected):
+    """The remainder holds no word of the holiday's own name."""
+    got = extract_datetime(utterance, "en-US", REF)
+    assert got is not None
+    for word in utterance.split():
+        assert word not in got[1].lower()
+
+
+# --- a word of time beside the holiday name, from the #374 review ----------
+
+# The first cut compared the words the engine consumed against the words of the
+# holiday phrase, and declined whenever the engine consumed anything else. A
+# time of day is something else, so "good friday at 9am" kept the engine's
+# coming Friday and left `'good'` in the remainder: half the holiday's own name,
+# on the commonest spoken form of all eight. The test asks the engine the
+# holiday phrase on its own instead, and the override fires when that answer's
+# date equals the date the whole text produced, which a time of day does not
+# move.
+#
+# Each expected date is reckoned independently from the computus, as above.
+
+WEEKDAY_NAMED_HOLIDAYS_WITH_A_TIME = [
+    ("good friday at 9am", date(2027, 3, 26)),
+    ("remind me on good friday at 9am", date(2027, 3, 26)),
+    ("palm sunday at noon", date(2027, 3, 21)),
+    ("easter monday morning", date(2027, 3, 29)),
+    ("shrove tuesday at 6pm", date(2027, 2, 9)),
+    ("set an alarm for good friday at 7", date(2027, 3, 26)),
+]
+
+WEEKDAY_NAMED_HOLIDAY_WORDS = frozenset(
+    word for utterance, _ in WEEKDAY_NAMED_HOLIDAYS for word in utterance.split()
+)
+
+
+@pytest.mark.parametrize("utterance,expected",
+                         WEEKDAY_NAMED_HOLIDAYS_WITH_A_TIME)
+def test_a_time_beside_a_holiday_name_still_answers_with_the_holiday(
+        utterance, expected):
+    """A time of day beside the holiday name does not send the answer back to
+    the engine's weekday."""
+    got = extract_datetime(utterance, "en-US", REF)
+    assert got is not None
+    assert got[0].date() == expected
+
+
+@pytest.mark.parametrize("utterance,expected",
+                         WEEKDAY_NAMED_HOLIDAYS_WITH_A_TIME)
+def test_a_time_beside_a_holiday_name_keeps_no_half_of_the_name(
+        utterance, expected):
+    """The remainder holds no word of any weekday-named holiday.
+
+    The whole set is checked, not only the words of this utterance, because the
+    failure this cell exists for left `'good'`, `'palm'`, `'easter'` and
+    `'shrove'` behind.
+    """
+    got = extract_datetime(utterance, "en-US", REF)
+    assert got is not None
+    remainder_words = set(got[1].lower().split())
+    assert not (remainder_words & WEEKDAY_NAMED_HOLIDAY_WORDS)
+
+
+def test_a_time_beside_a_holiday_name_is_left_in_the_remainder():
+    """Known answer, and the bound on this round: the holiday layer answers
+    with a date, so the engine's time of day is not carried onto it.
+
+    "good friday at 9am" gives Good Friday at midnight and leaves "at 9am" in
+    the remainder. The time is not lost, but a caller that reads only the
+    datetime sees midnight. Carrying it would also have to take the time words
+    out of the remainder, which is a second change this round does not make.
+    The day the time is carried this cell fails and says so.
+    """
+    got = extract_datetime("good friday at 9am", "en-US", REF)
+    assert got is not None
+    assert got[0].date() == date(2027, 3, 26)
+    assert got[0].time() == time(0, 0)
+    assert "9am" in got[1]
+
+
+def test_a_plain_weekday_still_answers_with_the_weekday(utterance=None):
+    """Control: a weekday that names no holiday is untouched.
+
+    25 September 2026 is a Friday, so the coming Sunday is the 27th.
+    """
+    got = extract_datetime("sunday", "en-US", REF)
+    assert got is not None
+    assert got[0].date() == date(2026, 9, 27)
+
+
+def test_a_weekday_beside_a_holiday_keeps_the_weekday():
+    """Control: the engine's answer stands when it read words the holiday
+    phrase does not cover.
+
+    "play christmas music on friday" asks about Friday. The holiday phrase is
+    "christmas" alone, so the engine's Friday is not inside it and the holiday
+    date must not replace it.
+    """
+    got = extract_datetime("play christmas music on friday", "en-US", REF)
+    assert got is not None
+    assert got[0].date() == date(2026, 9, 25) or got[0].date() == date(2026, 10, 2)
+    assert got[0].date() != date(2026, 12, 25)
+
+
+def test_a_holiday_the_engine_cannot_read_is_unchanged():
+    """Control: the None path of the engine still answers from the layer."""
+    got = extract_datetime("boxing day", "en-US", REF)
+    assert got is not None
+    assert got[0].date() == date(2026, 12, 26)
+
+
+# --- the bound on the extent test, from the #374 review ---------------------
+
+# `holiday_overrides_engine` compares a set of folded words, not a position, so
+# a weekday word that appears both inside the holiday name and elsewhere in the
+# sentence is indistinguishable from one that appears inside the name alone. A
+# relative phrase built on such a word answers with the holiday itself.
+#
+# These cells record the answer this library gives today, not the answer the
+# phrase asks for. Each expected date is reckoned independently: Western Easter
+# 2027 is 28 March by the Gregorian computus, so Good Friday is 26 March and
+# Easter Monday is 29 March. The date the phrase actually names is written
+# beside each cell. The day the extent test becomes positional these cells fail
+# and say what changed.
+
+KNOWN_ANSWER_RELATIVE_PHRASES = [
+    # utterance, the date given today, the date the phrase names
+    ("the friday before good friday", date(2027, 3, 26), date(2027, 3, 19)),
+    ("monday after easter monday", date(2027, 3, 29), date(2027, 4, 5)),
+]
+
+
+@pytest.mark.parametrize("utterance,given,asked",
+                         KNOWN_ANSWER_RELATIVE_PHRASES)
+def test_a_relative_phrase_on_a_holiday_word_answers_with_the_holiday(
+        utterance, given, asked):
+    """Known answer: the holiday's own date, not the day the phrase names.
+
+    The engine gave the coming weekday for these phrases before this fix
+    existed, which was wrong too, so the cell bounds the fix rather than
+    approving it.
+    """
+    got = extract_datetime(utterance, "en-US", REF)
+    assert got is not None
+    assert got[0].date() == given
+    assert got[0].date() != asked
+
+
+def test_a_relative_phrase_the_engine_keeps_is_not_taken():
+    """Control, the opposite call on a phrase of the same shape.
+
+    "good friday and next friday" carries a Friday outside the holiday phrase,
+    so the consumed words are not a subset of the phrase and the engine keeps
+    the utterance. 25 September 2026 is a Friday, so the next one is 2 October.
+    """
+    got = extract_datetime("good friday and next friday", "en-US", REF)
+    assert got is not None
+    assert got[0].date() == date(2026, 10, 2)