Answer the day and the city actually asked about (#388) #31

2026-07-31T12:06:00+02:00

claude commented

2026-07-31 12:06:00 +02:00

Three commits. Vikunja #388 — the two wrong answers PR 30 made reachable, plus #389.

"какое число завтра" answered today. The arm keyword-matched "числ" and threw the rest of the question away. It now reads the day, using ParseCalendarDate — the parser the neighbouring calendar arm already trusts — extended with послезавтра and вчера rather than growing a second day table in the reply code. A fourth place that parses Russian day words is how this bug happened in the first place.

That parser also had a real bug of its own: it cut the day boundary with Truncate(24h), which truncates in UTC, so it returned the wrong day for any non-UTC local zone. Now time.Date in the local zone.

"который час в киеве" answered local time. Now any named place gets an honest answer instead:

я знаю только местное время, про другие города пока не скажу.

Guards for "в данный момент" and "в 5 часов" so those aren't mistaken for places.

The third commit is why it stops there. The first version answered the named city's time from a city→zone table. You said you'll never ask the time outside your own zone, so that table was a second list of cities to keep in step with the weather one, for a question you won't ask. It's deleted — the table, the lookup, and the embedded tz database. Closes #389.

The second commit closes the same hole the first one left open. The city arm got an honest fallback; the date arm didn't. "какое число в пятницу" still silently answered today, because the parser only knows ±2 days and the code treated "couldn't parse" as "he meant today". That is the exact bug this PR is about, one step out. So:

я считаю только сегодня, завтра, послезавтра и вчера — про другие дни пока не скажу.

And one line found on the way: the calendar arm called time.Now() instead of the handler's injected clock, so it ignored the test clock and couldn't be tested at a fixed time. Which is how an arm like this stays broken.

Tests use a fixed clock — Thursday 30 July 2026 — and assert the actual date string, not that some reply came back: today, завтра, послезавтра, вчера, three unresolvable days, plus four named places all getting the local-time-only answer. Asserting "a reply exists" is how the original bug passed for so long.

One thing left alone: the other replySystem arms are all "not wired yet" placeholders, so none of them has this bug.

Three commits. Vikunja #388 — the two wrong answers PR 30 made reachable, plus #389. **"какое число завтра" answered today.** The arm keyword-matched "числ" and threw the rest of the question away. It now reads the day, using `ParseCalendarDate` — the parser the neighbouring calendar arm already trusts — extended with `послезавтра` and `вчера` rather than growing a second day table in the reply code. A fourth place that parses Russian day words is how this bug happened in the first place. That parser also had a real bug of its own: it cut the day boundary with `Truncate(24h)`, which truncates in **UTC**, so it returned the wrong day for any non-UTC local zone. Now `time.Date` in the local zone. **"который час в киеве" answered local time.** Now any named place gets an honest answer instead: > я знаю только местное время, про другие города пока не скажу. Guards for "в данный момент" and "в 5 часов" so those aren't mistaken for places. **The third commit is why it stops there.** The first version answered the named city's time from a city→zone table. You said you'll never ask the time outside your own zone, so that table was a second list of cities to keep in step with the weather one, for a question you won't ask. It's deleted — the table, the lookup, and the embedded tz database. Closes #389. **The second commit closes the same hole the first one left open.** The city arm got an honest fallback; the date arm didn't. "какое число в пятницу" still silently answered **today**, because the parser only knows ±2 days and the code treated "couldn't parse" as "he meant today". That is the exact bug this PR is about, one step out. So: > я считаю только сегодня, завтра, послезавтра и вчера — про другие дни пока не скажу. **And one line found on the way:** the calendar arm called `time.Now()` instead of the handler's injected clock, so it ignored the test clock and couldn't be tested at a fixed time. Which is how an arm like this stays broken. **Tests** use a fixed clock — Thursday 30 July 2026 — and assert the actual date string, not that some reply came back: today, завтра, послезавтра, вчера, three unresolvable days, plus four named places all getting the local-time-only answer. Asserting "a reply exists" is how the original bug passed for so long. **One thing left alone:** the other `replySystem` arms are all "not wired yet" placeholders, so none of them has this bug.

claude added 3 commits 2026-07-31 12:06:01 +02:00

Answer the day the user asked about and the city he named (#388 ) d00929ac0b

replySystem had two arms that PR 30 made reachable, and both answered confidently wrong: the date arm keyword-matched "числ" and always answered today, so "какое число завтра" answered today; the clock arm ignored a named city and answered local time. The date arm now reads the day word through router.ParseCalendarDate (which grew послезавтра/вчера and now cuts the day boundary in the local zone instead of UTC). The clock arm answers the named zone when it resolves offline from the tz database embedded in the binary, and otherwise says plainly that she only knows local time.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGeSZxh1DCtRxmFVSYVGvJ

Merge the system reply fixes f179ae2fde

Say so when the day asked about is out of reach 84ba217892

kami added 1 commit 2026-07-31 12:14:20 +02:00

Drop the city time-zone table 3dbf67f8f9

The user only ever asks the time in his own zone, so answering other
cities was code kept in step with the weather city list for no gain.
Any named place now gets the honest "local time only" answer that was
already there for unknown cities.

Removes the 22-entry table, the lookup and the embedded tz database.
Closes Vikunja #389 — there is only one city list again.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01CGeSZxh1DCtRxmFVSYVGvJ

kami commented

2026-07-31 20:22:22 +02:00

Superseded by #47, which landed this whole stack on master as one reviewed integration merge. This PR head is an ancestor of master — its commits are in, nothing here is lost. Closing as merged-by-proxy rather than merged, since the merge came in through #47.

Review threads on this PR were answered or acted on before the merge; the Russian wording fixes went in as #48.

Superseded by #47, which landed this whole stack on master as one reviewed integration merge. This PR head is an ancestor of master — its commits are in, nothing here is lost. Closing as merged-by-proxy rather than merged, since the merge came in through #47. Review threads on this PR were answered or acted on before the merge; the Russian wording fixes went in as #48.

kami closed this pull request

2026-07-31 20:22:23 +02:00

Pull request closed

Please reopen this pull request to perform a merge.

Sign in to join this conversation.

2 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: kami/Maven#31