Add model evaluation run to store and view

[charisk]

This PR introduces the ModelEvalutionRun domain entity and wires up model evaluation run information to the modeling store and the model editor view. Some notes:

• The ModelEvaluationRun interface is pretty minimal at the moment. I expect it to grow while we build more features (e.g. it might need to contain model packs).
• The model evaluation run information is not yet persisted - it's only in memory (in the ModelingStore).
• A new ModelEvaluator class has been added to contain logic around model evaluation in order to keep changes in model-editor-view minimal. The implementation is pretty minimal for now and will be fleshed out in the near future.

Checklist

N/A:

• CHANGELOG.md has been updated to incorporate all user visible changes made by this pull request.
• Issues have been created for any UI or other user-facing changes made by this pull request.
• [Maintainers only] If this pull request makes user-facing changes that require documentation changes, open a corresponding docs pull request in the github/codeql repo and add the ready-for-doc-review label there.

[robertbrignull]

I'm wondering are we duplicating too much data from the variant analysis status?

Whenever the variant analysis run changes status from in-progress to some other state, we have to see that event and then update the state here. I worry it could lead to bugs if the two states get out of sync.

An alternative could be essentially ModelEvaluationStatus = "preparing" | "delegateToVariantAnalysisStatus" (but with better names).

Is this something you considered? Apologies I haven't been following the earlier PRs or issues closely so I likely missed any discussion there.

[charisk]

This is a valid point. I did consider it, but because VariantAnalysisStatus is an enum it's quite awkward - you can't just extend it. And since the variant analysis statuses don't really change (since they come from the back-end), it felt safe enough to re-define them. If we want to go down that route, I would suggest we change VariantAnalysisStatus to use strings rather than enums.

The way the type is created right now also comes with more flexibility about what states we want to expose for model evaluation runs.

[robertbrignull]

What you've said makes perfect sense, but it wasn't quite what I was suggesting. Apologies I didn't make myself clear enough.

It being an enum does indeed make it hard to extend, and I don't have any objection to redefining the type for passing around transient values internally. My point was rather should we be storing that type anywhere for any length of time (even in memory, let alone to disk).

What I was trying to describe wasn't to "extend" the VariantAnalysisStatus type, but rather that one of the values in ModelEvaluationStatus means "I don't know. Go check the variant analysis status instead". By doing this it means we don't have to worry about keeping the value up to date whenever the variant analysis status changes, and instead we go calculate it when needed.

Therefore the logic for getting the model evaluation status would look like:

getModelEvalationStatus(): InternalModelEvaluationStatus {
  if (modelEvaluationRun.status === "preparing") {
    // It's preparing still and the variant analysis hasn't been triggered yet
    return "preparing";
  } else {
    // Ask the variant analysis manger for the status and process / return that
    const variantAnalysis = variantAnalysisManager.getVariantAnalysis(modelEvaluationRun.variantAnalysisId);
    if (variantAnalysis.status === "in_progress") {
      return "in_progress";
    } else if (variantAnalysis.status === "canceled") {
      return "canceled";
    } else if (...) {
      etc...
    }
  }
}

[charisk]

Oh I see! That's interesting - it's not something I considered. Do you mind if we discuss this separately (we have an issue to track work around domain entities)? Seems like something that would be good to discuss with @shati-patel when she's back, and ideally I wouldn't like it to block this PR.

[robertbrignull]

Sure, we can discuss on that issue. Although this PR is introducing domain entities itself, the code is still very easy to change at this stage. I'm happy to discuss on that issue instead.

[charisk]

I've made some updates following our discussion.

[charisk]

I will do this in the next PR. It needs the VariantAnalysisManager wired in and it would make the PR bigger than it is so I left it out for now.

[shati-patel]

Looks good ✨ Just a few minor comments!

[shati-patel]

Good stuff 💪🏽 😀