Update method sort order to place auto-model predictions earlier

[robertbrignull]

As described in the code, the intended sort order is:

• Unsaved positive AutoModel predictions
• Negative AutoModel predictions
• Unsaved manual models + unmodeled methods
• Saved models from this model pack (AutoModel and manual)
• Methods not modelable in this model pack

Then secondarily on the number of usages and the signature, as we do currently.

This required passing a lot more information into the sorting function, which has made the code a little bit more complex.

I've also added unit tests of the sorting function. I couldn't find any existing methods, but I think we definitely needed some now.

Also includes a couple a small refactorings (done as their own commits) to make the later changes easier.

Checklist

• 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 haven't had time to manually test this yet, but I wanted to get the PR up before I finish today. I'll do more testing of this tomorrow to make sure it's all working.

[robertbrignull]

I've pushed changes to the sort order. There are still some slight rough edges:

• If you edit a negative AI prediction it'll suddenly get sorted higher with the positive AI predictions. Because it now has a modeling, and processedByAutoModelMethods is still set even though you edited it manually. I don't think I ever want the sort order to immediately change as a result of me changing a dropdown because then I can't find the method again. Maybe this is enough of an edge case that we can leave it for now.
• When you change a method from unmodeled to manually modeled it flickers as it sorts to the bottom and then back to where it was. I think this is because the modeledMethod and modifiedSignatures get set separately and there's a render in between them. This one might be possible to fix but I haven't look into it yet. Do you think it's ok to leave this for a followup PR or should we do it in this PR?

[robertbrignull]

• When you change a method from unmodeled to manually modeled it flickers as it sorts to the bottom and then back to where it was. I think this is because the modeledMethod and modifiedSignatures get set separately and there's a render in between them. This one might be possible to fix but I haven't look into it yet.

I've looked into this now and I think it can be fixed but it'll be quite a big change and definitely not something to do lightly.

In ModelEditorView.addModeledMethods we call modelingStore.addModeledMethods and then modelingStore.addModifiedMethods as two separate steps, and each of these sends an individual message to the view to update its state. With this approach it's impossible to make the updates to the view state at the same time.

One option could be to combine the modeling store updates into one method, and then also combine the view state updates into one message. So instead of having separate setModeledMethods and setModifiedMethods messages, we'd combine those and maybe others into a generic setState message that could update multiple pieces of state at once.

Unfortunately this is quite a bit change to how we organise our webview messages.

[koesie10]

• If you edit a negative AI prediction it'll suddenly get sorted higher with the positive AI predictions. Because it now has a modeling, and processedByAutoModelMethods is still set even though you edited it manually. I don't think I ever want the sort order to immediately change as a result of me changing a dropdown because then I can't find the method again. Maybe this is enough of an edge case that we can leave it for now.

I think this is actually quite common when the LLM missing something that is a genuine sink/source. However, I'm not sure how we can fix this without changing something about our data model, so perhaps we can leave this for now and try to fix it in a follow-up?

I've looked into this now and I think it can be fixed but it'll be quite a big change and definitely not something to do lightly.

I agree, and depending on how jarring it is, we should try to fix this in a follow-up. A somewhat easier change might be to delay sending postMessage calls for 1 "tick" (i.e. setTimeout(fn, 0) and adding some mechanism to receive batch messages in the webview. Then we wouldn't need to change the webview messages and could instead just rely on that logic to handle it (and React should batch those state updates as well).

I think none of these are blockers and that we can merge this as-is (I haven't run this locally yet).

[robertbrignull]

A somewhat easier change might be to delay sending postMessage calls for 1 "tick" (i.e. setTimeout(fn, 0) and adding some mechanism to receive batch messages in the webview. Then we wouldn't need to change the webview messages and could instead just rely on that logic to handle it (and React should batch those state updates as well).

That does sound like a simpler change. Theoretically it should be safe to batch messages and maybe it'll improve performance in other places too.

If we merge this as it is then I can open issues the next steps of editing negative AI predictions and the flickering when adding the first modeling of a method.