[editor] Support Updating AIConfig State when aiconfig Prop Changes

[rholinshead]

[editor] Support Updating AIConfig State when aiconfig Prop Changes

Reviving the changes closed in #960 so that we can leverage the aiconfig prop to the AIConfigEditor component for updating the config in state. This will be useful for 2 reason in the vscode extension:

1. Currently, re-saturating the webview has no aiconfig in state. This causes an empty webview (with loading spinner) in the case where the server has issues and can't load the content (e.g. if the config JSON has invalid model parsers defined at the top). We can improve that UX to have a readonly copy of the config by saving the aiconfig if webview context to re-saturate with. The problem with doing that right now is that the /load call after the editor renders would not update the editor with the loaded config, which means it could be showing a stale config. This PR would fix that
2. Undo/redo. Similar to the above, in vscode extension undo/redo is controlled by the webview extension host and updates the server with the updated state. This then propagates the updated aiconfig to the webview client, which currently does nothing with the updated config. This PR would fix that as well

An important note here:

When an updated aiconfig prop is passed, we just replace the entire aiconfig state in the editor, instead of consolidating with existing state. This will ensure:

• new prompt keys are set in the _ui, meaning the prompts are remounted (so, undo/redo will replace the input properly)
• config passed through is the source of truth, instead of having a weird case where client state is persisted / overwriting the provided config, causing incorrect state

We can consider the scenario of passing updated aiconfig prop through as treating the editor as a controlled component, at least temporarily. As a 'user' of the component, one would expect passing a new aiconfig prop would make the editor have that exact contents.

Lastly, just noting we could do some smarter consolidation using lodash deep equality checks in order to retain existing prompts that are unchanged when passing a new aiconfig prop through (so, they'd keep their _ui.id and not be remounted). Just noting that there would probably be no perf benefit (since deep equality check may be just as expensive as rerendering the components, if not more). The only benefit could be in improving the UX if the current implementation causes weirdness. For example, in the test video there is some flickering of the audio input as it re-renders, but I'm not sure if that warrants the expensive equality checks on its own

Testing:

Updated LocalEditor to leverage state instead of callbacks for streaming:

output_chunk: (data) => {
            //onStream({ type: "output_chunk", data: data as Output });
            const output = data as Output;
            setAiConfig(
              (prev) =>
                ({
                  ...prev,
                  prompts: prev!.prompts.map((prompt) => {
                    if (prompt.name === "get_activities") {
                      return { ...prompt, outputs: [output] };
                    }
                    return prompt;
                  }),
                } as AIConfig)
            );
          },
          aiconfig_chunk: (data) => {
            //onStream({ type: "aiconfig_chunk", data: data as AIConfig });
            setAiConfig(data as AIConfig);
          },

Then make sure streaming works

Screen.Recording.2024-02-20.at.11.07.46.AM.mov

Note that in the video I'm also clicking into the input to show that it's continuously re-rendered, so user cannot actually click and edit it while the state is changing from the streaming

[saqadri]

Incredible writeup of the various cases and options considered. This change looks good to me. Main question -- did you check this in the vscode editor? I tried this from #960 when I was building the VSCode editor, and it didn't seem to propagate updates properly. However, it's possible that some of the other fixes you made to re-render as state changes were the missing piece.

[rholinshead]

Incredible writeup of the various cases and options considered. This change looks good to me. Main question -- did you check this in the vscode editor? I tried this from #960 when I was building the VSCode editor, and it didn't seem to propagate updates properly. However, it's possible that some of the other fixes you made to re-render as state changes were the missing piece.

Ah, no I didn't try it out since I'm not sure the best way to do that besides pulling my aiconfig-editor package PR and rebasing this on top and linking it; how did you test out #960 before?
Fwiw, the change of overwriting the state (mainly, the fact that the ClientPrompts have new ids/keys) is what will 'fix' the issue you saw

[saqadri]

how did you test out #960 before?

With difficulty. I copied the VSCodeEditor.tsx (and related files) into the aiconfig-editor folder, built the main.js bundle from there, and copied it into the vscode-extension folder lol. Probably overkill and better to just publish a package update.

One thing I'd suggest is we should have a test-aiconfig-editor package on npm that we update to test stuff out as needed. But also feel free to push an update to the main package directly right now to test it out.

[rholinshead]

how did you test out #960 before?

With difficulty. I copied the VSCodeEditor.tsx (and related files) into the aiconfig-editor folder, built the main.js bundle from there, and copied it into the vscode-extension folder lol. Probably overkill and better to just publish a package update.

One thing I'd suggest is we should have a test-aiconfig-editor package on npm that we update to test stuff out as needed. But also feel free to push an update to the main package directly right now to test it out.

Ah, ok, that's what I feared :s

I was planning to publish and bump the minor version with this since it does add some new behaviour that changes how the editor behaves.

I can publish to a test package and update test plan for vscode, but will land first so it's easier to do that without dealing with additional merge issues