Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Infer date/location from context #105

Open
dcastro opened this issue Sep 6, 2023 · 2 comments
Open

Infer date/location from context #105

dcastro opened this issue Sep 6, 2023 · 2 comments
Milestone

Comments

@dcastro
Copy link
Member

dcastro commented Sep 6, 2023

Clarification and motivation

The parser tries its best to parse timer references along with date/location references.

Let's meet at 10am MSK tomorrow
Time ref = 10am, date ref = tomorrow, location ref = MSK

However, sometimes the date/location refs can be located far from the time reference:

How about Thursday? Let's say, 10am?
Tomorrow, I'll be able to connect between 10am and 11am

There was an attempt In #82 to address this concern but, IMO, it went a bit too far.
It "scans" not only the user's message for date/location refs, but also previous messages in the chat as well:

Though I do agree with the intent, I have some concerns:

  1. It kills reproducibility. When a user reports an issue, and the message with the time references is sent to us, we won't be able to reproduce the issue based on that message alone.
  2. I'm a bit worried about other people's messages accidentally affecting the meaning of my messages.
  3. From a quick glance, I think the implementation of Remind contexts and apply to time refs #82 is based on caching time references from messages in a thread. But when the server is restarted, the cache is lost. Which means the result of a conversion would depend on yet another unpredictable variable, "how long the server has been up".

IMO, we should reuse parts of that PR, but we should restrict the behavior to the sender's own message only.

@dcastro dcastro added this to the v1.1 milestone Sep 7, 2023
@dcastro
Copy link
Member Author

dcastro commented Sep 20, 2023

Another example, "tomorrow" was not parsed.

image

@PrestoConFuoco1
Copy link

PrestoConFuoco1 commented Nov 30, 2023

From a quick glance, I think the implementation of #82 is based on caching time references from messages in a thread. But when the server is restarted, the cache is lost. Which means the result of a conversion would depend on yet another unpredictable variable, "how long the server has been up".

Hm, how about just loading the cache for that thread again when required? At least we will stay with reproducibility on the thread level. Of course, for single messages reproducibility will be lost, but I believe that we'll win enough to put up with it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants