[Regression] Data in compare_commitcomparison table is stale, leading to misleading patch coverage info on the UI

[rohan-at-sentry]

Describe the bug Users (mostly those that upload multiple coverage reports per commit) are reporting seeing stale patch coverage on the UI.

To Reproduce See PR here --> https://github.com/vlad-ko/laravel-stripe-app-gh/pull/58

@vlad-ko can add more color - we saw that the patch coverage was being displayed correctly by the annotation / browser plugin (anything using the public API to pull info from our DB).

Looking at the compare_commitcomparison table in DB for the commit ID shows data that is stale (null in db) from the real value (100%).

Expected behavior Data consistent with reality.

[rohan-at-sentry]

@vlad-ko @drazisil-codecov can you please add more examples here.

[drazisil-codecov]

https://app.codecov.io/gh/rustymotors/server/commit/98fd4ba35a0da187cae71ea23295ba5413f1f039

https://github.com/rustymotors/server/commit/98fd4ba35a0da187cae71ea23295ba5413f1f039

^^ This SHA, as an example seems to be in disagreement about ptch coverage.

==

https://app.codecov.io/gh/rustymotors/server/commit/7cad266e8230cd7a719298ed0763b778a4577ce1

https://github.com/rustymotors/server/commit/7cad266e8230cd7a719298ed0763b778a4577ce1

^^ This SHA was almost 100% only tests, and while the UI seems to realize this (despite two errored reports (All uploaded reports are identical, except for flags)), the status check does not agree.

==

https://app.codecov.io/gh/rustymotors/server/pull/2037/commits

Apparently , 99% of the commits for this PR have patch coverage, which is incorrect. Might be unrelated to the rest of the examples though, unclear.

[jerrodcodecov]

Another example here: If we have the head coverage, then by definition we should have a patch coverage as well

Relevant commit SHA:

4213aec7afab787d42252ff61e86674d43e7baf

[giovanni-guidini]

Random notes

• Comparisons are created in the manual_trigger and by UploadFinisherTask
• Accessing the ImpactedFiles in the UI triggers a new compute comparison task (this has to be the case for this example because it only has carryforward reports) (this might explain why the example in the description is now displaying 100% patch in the UI)
• Comparison is updated by the ComputeComparison task
• Comparison requires both a BASE and a HEAD commit (to get the diff).
  • verify if Comparison requires BASE to have a report attached to it as well --> YES (PR up to change this assumption)
• ~SyncPullTask can create a Pull object without a HEAD commit (see here)~ (seems irrelevant)
• ComputeComparison fail rate seems to be around 1.3%. It is a slow task.
• In order for ComputeComparisonTask to run via UploadFinisherTask the notifications need to be called (1) and the commit needs to be part of a PR (2). I think these assumptions are correct. (1) because if the notification is not to be called we probably have more reports being processed and (2) because if there's no PR then we don't have a BASE for the comparison.

[giovanni-guidini]

Small update

@JerrySentry and I have looked quite a bit into this at this point. We did agree that the Comparison didn't needed a BASE report to have patch coverage, and that has been fixed. Now we should get patch info even if the BASE REPORT is missing (base commit missing doesn't get patch coverage because there's no git diff to apply)

Some examples we checked are explained by:

The original example fixed itself after a while. The ComputeComparison task ran twice. We believe the 1st run indicated no coverage because that upload didn't include any info in the diff (so it looks like coverage was not affected by patch). Many (6) hours later the task ran again, and this time the patch coverage was updated. We believe this has to do with a known issue in which UploadProcessor tasks "disappear" and "appear" again after 6h.

There was another instance in which the PR view displayed patch coverage, but the commit view didn't. We noticed there were 2 comparisons, and the commit one was correctly saying the coverage was not affected by the patch, because the uploads never included the files in the patch.

We will review more examples in the coming days, but nothing seems terribly broken at this point (other than what was mentioned before).