[editor] Use Prompt ID Instead of Index for Client-side State Actions #666
Conversation
rholinshead
requested review from
saqadri,
suyoglastmileai,
Ankush-lastmile,
jonathanlastmileai and
rossdanlm
as code owners
| case "ADD_PROMPT_AT_INDEX": { | ||
| return reduceReplacePrompt( | ||
| state, | ||
| action.prompt._ui.id, |
There was a problem hiding this comment.
Note that this action doesn't have an index explicitly passed in on dispatch, so need to grab it from the newly-inserted prompt in the action
# [editor] Model Selector Implement the model selector and add a placeholder call to the future api endpoint for updating the model. https://github.com/lastmile-ai/aiconfig/assets/5060851/812f3a10-aaac-4ec3-9f1f-4d21ebc99e9f
# [editor] Support Deleting Prompt Add the UI side for deleting a prompt, with placeholder to call the server endpoint once it's implemented https://github.com/lastmile-ai/aiconfig/assets/5060851/b3e682dd-a3ed-45c1-b153-b4b6e2094616
# [editor] Use Prompt ID Instead of Index for Client-side State Actions We can uniquely identify prompts in client-side state using their client-side id, so let's use that instead of index for referencing them in the actions. This should be a bit more robust than using index. ## Testing: Went through all relevant actions and made sure they work as expected
[editor] Support Deleting Prompt # [editor] Support Deleting Prompt Add the UI side for deleting a prompt, with placeholder to call the server endpoint once it's implemented https://github.com/lastmile-ai/aiconfig/assets/5060851/b3e682dd-a3ed-45c1-b153-b4b6e2094616 --- Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/lastmile-ai/aiconfig/pull/665). * #667 * #666 * __->__ #665 * #662
| @@ -96,7 +96,7 @@ export default function EditorContainer({ | |||
| const onSave = useCallback(async () => { | |||
| setIsSaving(true); | |||
| try { | |||
| await callbacks.save(clientConfigToAIConfig(aiconfigState)); | |||
| await callbacks.save(clientConfigToAIConfig(stateRef.current)); | |||
There was a problem hiding this comment.
Why wouldn't we want to re-render if client aiconfig changes?
There was a problem hiding this comment.
We will always re-render if aiconfigState changes. This stateRef is a stable ref to use in the callbacks since otherwise using aiconfigState in the callback means the callback logic is actually using the state from when the callback was constructed & memoized (which could technically be stale)
There was a problem hiding this comment.
Ah ok this makes sense! Why do we need to update for this diff? Is it because the index before was always guaranteed to be correct as the index --> ui mapping?
There was a problem hiding this comment.
I don't necessarily know if it had to be update here, but it was a small enough change and related to using proper source of truth (like id instead of index)
| ...responsePrompt, | ||
| ...statePrompt, |
There was a problem hiding this comment.
Just to confirm, this means we use the fields from responsePrompt first and whatever is left over that hasn't been defined by responsePrompt and exists within `statePrompts, that's what they're going to use?
[editor] Run Prompt (non-streaming) # [editor] Run Prompt (non-streaming) With non-streaming /run endpoint properly returning the response now, we can update the client config with the resulting output. We can also leverage the client-side `_ui` state for maintaining 'isRunning' state to show a spinner / disable the execute button for now (TODO: we may need to handle canceling execution as well). https://github.com/lastmile-ai/aiconfig/assets/5060851/67fa793a-9a60-4285-a7d1-69fff633d6a6 --- Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/lastmile-ai/aiconfig/pull/667). * __->__ #667 * #666 * #665 * #662
[editor] Use Prompt ID Instead of Index for Client-side State Actions
[editor] Use Prompt ID Instead of Index for Client-side State Actions
We can uniquely identify prompts in client-side state using their client-side id, so let's use that instead of index for referencing them in the actions. This should be a bit more robust than using index.
Testing:
Went through all relevant actions and made sure they work as expected
Stack created with Sapling. Best reviewed with ReviewStack.