Improve accuracy of target document after interval turnover

[kennsippell]

Is your feature request related to a problem? Please describe.
Target documents have a monthly interval. Because target documents are not always written (max of once per day, and only when viewing targets tab) they can be stale. When the interval ends, there is some "lazy best effort" to ensure that the previous interval's data is updated.

The digital payment system relies on the data in the previous month's target document. This month, there was a lot of variance between the "ideal" result (what would actually be calculated by targets.js for the month) vs the "actual" data written to the target document (the result of this interval turnover).

This feature suggests improvements to target documents so at least the previous month's target document is guaranteed to be the result of some deterministic predictable calculation.

Describe the solution you'd like
Can we recalculate all emissions during the interval turnover process before writing the values?
Can we run the interval turnover function even if the previous month's target document doesn't exist?
Or could we update the target document every time rules emissions are updated instead of daily when targets are viewed?

Additional context
Some scenarios

1. Does activity on 24th of the month. Then views a different contact on 1st of next month without going to tasks tab. Their activity is not included in the target document.
2. User does activity on 30th of the month. Views the tasks tab. This will work great! Except user changes devices on the 1st of the month and interval turnover happens on the new device (actual scenario from telemetry)

[dianabarsan]

(max of once per day, and only when viewing targets tab)

One mention here that reloading the app will also write the doc to disk.

Can we recalculate all emissions during the interval turnover process before writing the values?

Unfortunately, because a lot of targets emit "now", the emission date for the target when we recalculate will be on the next interval, and this will result in a 0 target count for the past interval.

Can we run the interval turnover function even if the previous month's target document doesn't exist?

I think this scenario can only exist when the user has not used the device during the past interval.
The interval turnover calculation uses the untouched stale rules engine cache to overwrite past month's target doc. If this cache does not exist, then nothing can be written.
Is an empty target document desirable here?

Or could we update the target document every time rules emissions are updated instead of daily when targets are viewed?

I believe this is the best and maybe only viable option.

[kennsippell]

One mention here that reloading the app will also write the doc to disk

So interval turnover happens when the app loads, but the routine writing of target documents doesn't trigger then. I think that's what you mean and we're on the same page?

Unfortunately, because a lot of targets emit "now"

Do you mean the target has date: 'now'? If we consider both potential values for date:

• date: 'reported' - As long as targets emit with an appropriate date, then it should be counted and filtered correctly. Digital Payments relies on two targets with this setting and so this is mostly the scenario I have been caring about during this investigation. AFAIK emissions (iff present) are counted properly even if refreshed after the interval turnover.
• date: 'now' - I think this setting is intended to "count all emissions regardless of their date." The risk I see here is that a report from the new interval could be counted in the previous interval. The interval turnover happens a lot ... but (for example), if the interval turns over and then a user creates a report via the reports tab, then they could erroneously add to a previous interval's target value. There are some other scenarios like this like documents created server-side.

I think this scenario can only exist when the user has not used the device during the past interval.

In Nairobi eCHIS Kenya, there were 400 users without an August target document. Of those, 51 had household visits or referrals in the month of August. I'm trying to understand this gap. According to Superset, some of these users had over 100 home visits and over 51 referrals. I'm hoping we can find why no target doc was written for these users. It seems to be responsible for about 15% of our digital payment data quality problems. My mistake. I thought this was a core issue, but was actually a result of multiple user accounts for the same facility_id and contact_id. Data is now consistent with the claim that this seems to only happen when the user is actually just totally not engaged.

I believe this is the best and maybe only viable option.

If memory serves me, the reason we didn't do this initially was that it may cause some noisey data synchronization and potentially conflicts if users login to multiple devices. If we are happy to ignore or address those concerns now, I think pursuing this would be a great solution for this issue.

[dianabarsan]

So interval turnover happens when the app loads, but the routine writing of target documents doesn't trigger then. I think that's what you mean and we're on the same page?

It does.

cht-core/shared-libs/rules-engine/src/provider-wireup.js

Line 295 in a2fe0b4

const handleIntervalTurnover = (provider, { monthStartDate }) => {

Do you mean the target has date: 'now'

Yes. When we "ask" the rules engine to give us emissions, targets with emission date of now will emit for the current date.
Then, we filter out all of the emissions that are not within our desired interval - this is how all targets calculations work:

cht-core/shared-libs/rules-engine/src/provider-wireup.js

Line 144 in a2fe0b4

const targetEmissionFilter = filterInterval && (emission => {

.
So when the interval turns over, we will essentially filter out ALL targets that emit for now, because they're not in the current interval.

may cause some noisy data synchronization and potentially conflicts if users login to multiple devices

This is correct. But to get higher accuracy, we need to commit these target docs more frequently.
I believe another issue would be the actual recalculation impact. For tasks we have a kind of "incremental" approach where we only recalculate tasks for a given contact, but when we recalculate targets, I don't think we have a way to reduce the calculation context to avoid doing a full run every time.

I'll be looking into this later today.

[dianabarsan]

I don't think we have a way to reduce the calculation context to avoid doing a full run every time.

We do indeed only recalculate targets for the updated context, not a full run every time.
And the target state gets updated whenever we refresh rules-engine state, this includes every time we recalculate tasks.
However, we only aggregate it when the user purposefully loads targets and on interval turnover.

[dianabarsan]

date: 'now' - I think this setting is intended to "count all emissions regardless of their date."

Correct, but the emission itself gets a timestamp as a date, the now is computed to someDate which happens to be the date when the emission is actually computed: https://github.com/medic/cht-conf/blob/main/src/nools/target-emitter.js#L30

So, if we're calculating on Jul 5th, all now emissions will emit for July 5th. And these will be excluded by a date filter that is for Jun 1 -> Jun 30 (previous interval).

[kennsippell]

Or could we update the target document every time rules emissions are updated instead of daily when targets are viewed? I believe this is the best and maybe only viable option.

Curious if your plan is to remove interval turnover as part of this change?

the emission itself gets a timestamp as a date

True, but also seems solvable. Could we add a flag to the emission like countAlways: true (or now: true etc) and only set the date for backward compatibility? Obv projects would need both a core and conf update to fix the bug, but it's maybe a clearer way to indicate the intent to "count all emissions regardless of their date".

If interval turnover is still required, this could maybe provide an alternate path to implementing this where-by we refresh emissions as a step during interval turnover.

[dianabarsan]

I think we have two problems here:

1. We only do a complete (all dirty contacts) recalculation:

• on startup after x seconds
• when viewing targets
• when viewing tasks

2. We can't retro-calculate because of how now is implemented

So we can't trust the rules state store to be up to date when we do the interval turnover, because of (1) and we can't recalculate because of (2).

I was dreaming about somehow recalculating the state on browser visibility change - so when the user puts the app in the background, but I'm afraid of indexeddb timeouts.
I don't have an answer of when is a good time to insert a recalculation of all dirty contacts that will resolve the turnover issue and insure the state is up to date.

Fixing (2) resolves scenario (1) from the original issue description.
Fixing (1) resolves scenario (2) from the original issue description.

I think we need both.

[korirm]

@dianabarsan MoH is keen to scale up digital payments work in Kenya by the end of November, and we feel that this will not be possible until we fix this data quality issue. Do we have estimated timelines that we can communicate to them?

[dianabarsan]

I'm currently working on this. I'm hoping this will be included in the next release (4.12). I wouldn't estimate it would take us beyond November to release. I feel positive about respecting your timeline.

[korirm]

@dianabarsan sorry I erred on my update it should have been beginning of November and not end of November.

[kennsippell]

@korirm This needs to be on user's devices by start of Nov. So we woiuld need to deploy probably no later than Oct 25th or thereabouts + working with Nairobi for a coordinated sync effort.

[dianabarsan]

I'm quite close to have this reviewed / smoke tested by the end of this week.

[dianabarsan]

FWIW the change involves recalculating tasks and targets immediately as changes occur instead as by-products of user actions or passing time. We would need to run a performance assessment to see if this is severely impactful (tagging @lorerod here, who was graciously volunteered to help, thanks @lorerod <3 ).

[dianabarsan]

This is now available for testing on 9431-always-aggregate .

[dianabarsan]

Merged to master and backported to 4.13.x