fix: a holiday offset written outside the construction moves the date - #381
Draft
openvoiceos-bot wants to merge 1 commit into
Draft
openvoiceos-bot wants to merge 1 commit into
openvoiceos-bot wants to merge 1 commit into
Conversation
`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
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Contributor
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: trueThanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Contributor
Hello there! Your PR checks are ready for review. ✨I've aggregated the results of the automated checks for this PR below. ⚖️ License CheckVerifying the origin of all contributed code. 🌍 ✅ No license violations found. Policy: Apache 2.0 (universal donor). StrongCopyleft / NetworkCopyleft / WeakCopyleft / Other / Error categories fail. MPL allowed. 📊 CoverageEnsuring every change is backed by a test. ✅ Files below 80% coverage (32 files)
Full report: download the 🔨 Build TestsChecking if the code is properly tempered. ⚔️ ✅ All versions pass
Helping you build the future of voice, one check at a time. 🎙️ |
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
2 participants
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.
What this changes
extract_holiday_spanparsedwritten[first:last], the characters the holidayconstruction itself matched, so a modifier written outside that construction
was never applied. The date was the bare holiday and the modifier came back in
the remainder as though it were question words.
Measured on
devat6bda4d9, anchor 25 September 2026 12:00:the day afterthe day beforetwo days afterle jour apresThe trace cannot decide it:
explainreports the same bareholiday_refoverthe holiday word alone for an offset phrase and for a question phrase, 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.remainderis read._holiday_extentstays, as the gate alone: a holiday reading is kept only whena winning match is a holiday construction. What the answer covers is then
chronologia's to say.
_caller_spellingmaps that remainder back onto the caller's own text._as_writtenmay put back an accent the caller never typed, and everysubstitution there replaces the same number of characters, so a position in one
string is the same position in the other. It 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.
After
The four rows answer 26, 24, 27 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, not read back from the parser.
The question rows
#372changed the parse input to protect are unchanged:"how many days until christmas" keeps
how many days until, "quantos diasfaltam para o natal" keeps
quantos dias faltam para o, "play some christmasmusic" keeps
play some music.Two things this does NOT fix, both filed and both with cells
"two days after christmas" still answers 27 September 2026 through
extract_datetime. The engine reads "two days" as an offset from the anchorand the walk never reaches the holiday layer, so the layer's correct answer is
never asked for. This is a different defect from the one above, and it is not
one
holiday_overrides_enginecovers: the engine consumed "two days", whichdoes not lie inside "christmas", so its subset rule correctly declines and must
not be widened to cover this. T-6961. The brief for this work recorded this
row's dev answer as 2026-12-25, which is the layer's answer; end to end it is
2026-09-27, and the table above says so.
The French "combien de jours avant noel" regresses, from 25 December,
correct, to 24 December, wrong, 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 noel" are both
read correctly, so the interrogative is what it turns on and not the language or
the preposition. Dev was right on that row by accident: the substring parse hid
the question's words from chronologia. Telling the two French shapes apart needs
a per-language table of interrogatives, which belongs to chronologia.
T-6896.
Two tests asserted that row. They are not deleted quietly: one drops the French
row and is renamed for what it now claims, the other is narrowed to the English
question it still proves, and a new cell holds the wrong answer explicitly with
its control and names the task. The day chronologia fixes it, that cell fails
and says so.
This is a trade, not a clean win: four wrong dates on the layer fixed, one
correct date made wrong. It is filed as a panel decision,
holiday-offset-vs-french-interrogative, rather than settled inside a commitbody.
It interacts with #374, which is in flight in the same files
#374 lands first and this branch rebases onto it. They merge cleanly by text:
git merge-treeover their merge base reports no conflict hunk.#374 added two known-answer cells holding the answers the library gave for a
relative phrase built on a weekday word inside a holiday name. They were
written to fail the day the extent stopped being a word-set comparison. This
branch is that day, and the answers they hold become the correct ones.
Western Easter 2027 is 28 March, so Good Friday is 26 March and the Friday
before it is 19 March; Easter Monday is 29 March and the Monday after it is
5 April. Both counted outside the library. On the combined tree the holiday
module gives 2 failed, 110 passed, and the two failures are exactly those
cells.
good fridaystill answers 26 March and the controlgood friday and next fridaystill answers 2 October from the engine.Whichever merges second updates those two cells to the right answer.
Suite
2906 passed, 3 skipped, 14 xfailed, 2294 subtests passed. The whole suite, not
a scoped module, because
holidays.pyis shared andextract_datetimeis thelibrary's entry point. The last recorded figure for dev is 2891 and this branch
adds fifteen cells; the skip, xfail and subtest totals do not move, so nothing
was converted to a skip.
With dev's
holidays.pycopied back over the fix, the module gives 16 failed,74 passed, so the new cells discriminate and are not a suite that would pass
against the implementation they were written to reject.
Evidence:
knowledge/wiki/audits/spec-adoption/t6895-c1-fix-forward.md🤖 Generated with Claude Code