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

Fix SKIP behavior in YES_AUTO and score #1916

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

KristianTashkov
Copy link
Contributor

@KristianTashkov KristianTashkov commented Jan 5, 2024

I've had some SKIPs over the Christmas holidays and noticed the score was more punishing than expected. I have a habit setup to be done three times every week. I expect that if I have skipped some days, they will be treated as non-existent days, and occurrences around them will connect. This is not what happens, though:
current
As you can see doing two occurrences and skipping the end of the week, doesn't count next week's occurence as the third in the chain, because it's too far away in the expected range. By changing the logic to treat skip_days as not existent days you can keep the habit going without being punished for having half-weeks + skips:
fixed

This affects both YES_AUTO and the actual score calculation.
@iSoron if you agree this should be fixed, I can update/write tests to cover it.

This behaviour has been mentioned before here:
#1686

@wobbba
Copy link
Contributor

wobbba commented Jan 5, 2024

I'mma randomly drop in to add my two cents:

Though I initially thought that the chain should connect as you described, I have since adapted my use of skip days as a half-check (in my head anyway). E.g. I meditated but only 5min instead of 15. Also like this, skip days seem OP imo, since you can create really long chains without actually doing anything. But from the name, it makes more sense to treat them as nonexistent. Maybe to be consistent we would then also need to exclude them from the chain length? So the chain in your second example would only be of length 22 instead of 28 (relevant for longest chain view).

Is this related to the issue where SKIP-ing a YES_AUTO day sometimes makes the score go down?

@KristianTashkov
Copy link
Contributor Author

KristianTashkov commented Jan 5, 2024

Maybe to be consistent we would then also need to exclude them from the chain length? So the chain in your second example would only be of length 22 instead of 28 (relevant for longest chain view).

I've suggested this before and @iSoron didn't like it since "chain length" is the length of the time period without a lapse, not how many times you did it persay.

Is this related to the issue where SKIP-ing a YES_AUTO day sometimes makes the score go down?

Yes! It fixes that problem as well :)
EDIT: Actually I'm not sure I have seen this one. The current code keeps the score on SKIP days the same as the previous one. But this PR fixes where score starts to go down after SKIP days even if you have enough occurrences

@wobbba
Copy link
Contributor

wobbba commented Jan 5, 2024

It was just something I noticed sometimes but here are steps to reproduce:

  • create 1 in 14 days yes/no habit
  • place a couple ticks (in my example 20/11/23, 12/12/23, 2/1/24)
  • check score (in my example 30%)
  • add skip day on current day (5/1/24)
  • score drops (29%)

This might be related to #1695 but that one seems closer to what you describe (it also mentions the issue you mentioned). In my case the skip day is within the implicitely checked range yet there is a drop in score. But possibly related. Do you think I should open another issue for this specifically?

@KristianTashkov
Copy link
Contributor Author

KristianTashkov commented Jan 5, 2024

Yes, that's a slightly different problem than #1695.

SKIP days keep the score the same as the previous day, but while a habit is still building, its score goes up on YES_AUTO days (while it converges to the occurrence counts you are doing). So, while adding a SKIP on those days doesn't decrease the score, it can reduce the rate of increase, leading to a lower score at the end.

An easy "fix" for this would be to make SKIP days take the higher of the "normally calculated" score or the previous score, but that goes against the "SKIP days are non-existent days" philosophy, which we seem to have. It would lead to score increasing indefinitely if you keep skipping and the week before the SKIPS you were doing well, which I don't think is something we want to have.

@wobbba
Copy link
Contributor

wobbba commented Jan 5, 2024

That makes sense! I agree, if we go for "skip days are nonexistent" then the behaviour I described is actually intended.

@iSoron
Copy link
Owner

iSoron commented Jan 31, 2024

Thanks for the PR, @KristianTashkov. I agree that we should treat SKIPs as non-existing days. This would be consistent with keeping the score unchanged when you skip a day. Could you please add some tests to verify the desired behavior? I think the example you have in this PR would be a good test.

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

Successfully merging this pull request may close these issues.

None yet

3 participants