New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Gen AI: separate "publish" and "save" buttons when filling out model card #57949
Conversation
() => dispatch(updateAiCustomization()), | ||
[dispatch] | ||
); | ||
const onSave = useCallback(() => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These multiple dispatches I believe are not the right way to do this, so looking for input :)
I see we have a couple instances where we put some redux updates that we want to happen after some async operation in this structure:
code-dot-org/apps/src/aichat/redux/aichatRedux.ts
Lines 318 to 331 in 194f7f0
extraReducers: builder => { | |
builder.addCase(submitChatMessage.fulfilled, state => { | |
state.isWaitingForChatResponse = false; | |
}); | |
builder.addCase(submitChatMessage.rejected, (state, action) => { | |
state.isWaitingForChatResponse = false; | |
state.chatMessageError = true; | |
console.error(action.error); | |
}); | |
builder.addCase(submitChatMessage.pending, state => { | |
state.isWaitingForChatResponse = true; | |
}); | |
}, | |
}); |
We also dispatch some redux actions inside of the thunk here:
code-dot-org/apps/src/aichat/redux/aichatRedux.ts
Lines 120 to 139 in 194f7f0
thunkAPI.dispatch( | |
setPreviouslySavedAiCustomizations(trimmedCurrentAiCustomizations) | |
); | |
const changedProperties = findChangedProperties( | |
previouslySavedAiCustomizations, | |
trimmedCurrentAiCustomizations | |
); | |
changedProperties.forEach(property => { | |
thunkAPI.dispatch( | |
addChatMessage({ | |
id: 0, | |
role: Role.MODEL_UPDATE, | |
chatMessageText: | |
AI_CUSTOMIZATIONS_LABELS[property as keyof AiCustomizations], | |
status: Status.OK, | |
timestamp: getCurrentTime(), | |
}) | |
); | |
}); |
Is one of these preferable? Is there a difference? Skimming through docs here, it seems like the first approach might be more standard?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like either approach is reasonable but defer to @sanchitmalhotra126 on a preference.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I mean nothing wrong with back to back dispatches as such, but if the goal here is that we only update the publish state after saving has completed, we should move this into the async thunk. In fact, if we need to save the publish state to the project, it might actually be better to do this in the opposite order - on clicking save, if the model card is not filled out, set hasPublished to false so that when you do save, that value is saved to the project.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check out the updates in the last two commits...there's at least one thing to figure out (I have an any
in there 😬 ), but I tried to refactor to use async thunks. I don't see the approach I have here (a shared bit of async code that takes in thunkAPI
as an arg), which usually isn't good. LMK what you think!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice updates! Just left a couple questions and one being a more of a product question. Thanks!
<div> | ||
{hasFilledOutModelCard(modelCardInfo) | ||
? renderPublishOkNotification() | ||
: renderCompleteToPublishNotification()} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if this is what others would expect, but it seems like once the model card is published the first time, that unless the model card is modified so that one of the fields is no longer filled out, it remains published but can be updated.
This implementation seems to imply that if a user publishes, and then updates the model card, the model card is then unpublished since message states 'Ready to publish'. Or if the user publishes then refreshes the browser, the model card is no longer published. Is that the intention? This is probably more of a product question @samantha-code .
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah good question, it's definitely a bit confusing. I was chatting with Sam about this yesterday and we were struggling a bit to come up with the right solution. Not sure I have the exact right answer, but I think we felt like this was "close enough" and we'd interate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think if we had different status messages based on whether you have a currently published model or not would be the right path? eg, "Ready to update published model" and "In order to update your published model, you must complete the model card"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I defer to @samantha-code but I think that different status messages would be helpful to the user.
I see Sanchit suggested making this a separate component #57949 (comment)
If this was a separate component, we could also add a warning for RetrievalCustomization
when a user adds/deletes a retrieval but still has to update/save.
const onPublish = useCallback(() => { | ||
dispatch(updateAiCustomization()); | ||
dispatch(setHasPublished(true)); | ||
dispatch(setViewMode(ViewMode.PRESENTATION)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When the user clicks on 'Publish' and then the 'User View' is displayed, I imagine that the user may want to immediately update Model Card fields and click on 'Edit Mode'. What do you think about saving state so that the 'Publish' tab and not 'Setup' remains open to encourage an iterative process? Also related to comment below, that 'User View' remains displayed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think about saving state so that the 'Publish' tab and not 'Setup' remains open to encourage an iterative process?
I think that makes sense! Currently, we don't manage the selected tab in redux, and the Tabs component I think would need to refactored to manage the selected tab as a prop. We also should probably switch over to using Denys's new Tabs component rather than my home baked one. Would it be ok to do that as a follow-up?
Also related to comment below, that 'User View' remains displayed?
Not sure I'm following this part, happy to chat offline if easier!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That would be great as a follow-up and +1 on using Denys's new Tabs component - thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also related to comment below, that 'User View' remains displayed?
Not sure I'm following this part, happy to chat offline if easier!
Ah sorry. I think once we save hasPublished
to sources, this will not be an issue when a user moves from level to level. I think I was referring to how when a model card has been published, the toggle remain visible.
() => dispatch(updateAiCustomization()), | ||
[dispatch] | ||
); | ||
const onSave = useCallback(() => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like either approach is reasonable but defer to @sanchitmalhotra126 on a preference.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work, this UI looks great! A few comments but nothing major.
One thing to consider in a follow-up maybe - what happens if you click publish and save back to back? Do we kick off two simultaneous saves? I don't think that's a huge deal, but it might be good to look into disabling one or the other button while a save is in progress?
() => dispatch(updateAiCustomization()), | ||
[dispatch] | ||
); | ||
const onSave = useCallback(() => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I mean nothing wrong with back to back dispatches as such, but if the goal here is that we only update the publish state after saving has completed, we should move this into the async thunk. In fact, if we need to save the publish state to the project, it might actually be better to do this in the opposite order - on clicking save, if the model card is not filled out, set hasPublished to false so that when you do save, that value is saved to the project.
return property === 'botName' ? 'input' : 'textarea'; | ||
}; | ||
|
||
const hasFilledOutModelCard = (modelCardInfo: ModelCardInfo) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be good to move this to a selector in aichatRedux since it's a state computation that directly drives some UI
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done (I think), LMK if this looks ok!
const currentAiCustomizations = useAppSelector( | ||
state => state.aichat.currentAiCustomizations | ||
const previouslySavedAiCustomizations = useAppSelector( | ||
state => state.aichat.previouslySavedAiCustomizations | ||
); | ||
const {systemPrompt, temperature, retrievalContexts} = | ||
currentAiCustomizations; | ||
const modelCardInfo = currentAiCustomizations.modelCardInfo; | ||
previouslySavedAiCustomizations; | ||
const modelCardInfo = previouslySavedAiCustomizations.modelCardInfo; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: could collapse this into const {systemPrompt, temperature, retrievalContexts, modelCardInfo} = useAppSelector(state => state.aichat.previouslySavedAiCustomizations);
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure looks like it, done!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just pushed a couple commits, LMK what y'all think!
<div> | ||
{hasFilledOutModelCard(modelCardInfo) | ||
? renderPublishOkNotification() | ||
: renderCompleteToPublishNotification()} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do you think if we had different status messages based on whether you have a currently published model or not would be the right path? eg, "Ready to update published model" and "In order to update your published model, you must complete the model card"?
return property === 'botName' ? 'input' : 'textarea'; | ||
}; | ||
|
||
const hasFilledOutModelCard = (modelCardInfo: ModelCardInfo) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done (I think), LMK if this looks ok!
const currentAiCustomizations = useAppSelector( | ||
state => state.aichat.currentAiCustomizations | ||
const previouslySavedAiCustomizations = useAppSelector( | ||
state => state.aichat.previouslySavedAiCustomizations | ||
); | ||
const {systemPrompt, temperature, retrievalContexts} = | ||
currentAiCustomizations; | ||
const modelCardInfo = currentAiCustomizations.modelCardInfo; | ||
previouslySavedAiCustomizations; | ||
const modelCardInfo = previouslySavedAiCustomizations.modelCardInfo; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure looks like it, done!
const onPublish = useCallback(() => { | ||
dispatch(updateAiCustomization()); | ||
dispatch(setHasPublished(true)); | ||
dispatch(setViewMode(ViewMode.PRESENTATION)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What do you think about saving state so that the 'Publish' tab and not 'Setup' remains open to encourage an iterative process?
I think that makes sense! Currently, we don't manage the selected tab in redux, and the Tabs component I think would need to refactored to manage the selected tab as a prop. We also should probably switch over to using Denys's new Tabs component rather than my home baked one. Would it be ok to do that as a follow-up?
Also related to comment below, that 'User View' remains displayed?
Not sure I'm following this part, happy to chat offline if easier!
() => dispatch(updateAiCustomization()), | ||
[dispatch] | ||
); | ||
const onSave = useCallback(() => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Check out the updates in the last two commits...there's at least one thing to figure out (I have an any
in there 😬 ), but I tried to refactor to use async thunks. I don't see the approach I have here (a shared bit of async code that takes in thunkAPI
as an arg), which usually isn't good. LMK what you think!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, nice refactoring! A few more small comments
apps/src/aichat/redux/aichatRedux.ts
Outdated
); | ||
|
||
// eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
const updateAiCustomizationShared = async (thunkAPI: any) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: maybe rename to updateAndSaveAiCustomizations
or just saveAiCustomizations
? Also I think if you just pass in state
and dispatch
as separate args, you should be able to get rid of the any
? Maybe even reduce down to just the specific state fields you need (current/previousAiCustomizations). We have a similar pattern in Lab2 where different async thunks make use of common shared functions, and just pass in the specific data they need from the thunkApi
. Ex:
code-dot-org/apps/src/lab2/lab2Redux.ts
Lines 388 to 396 in f0320b0
function setProjectAndLevelData( | |
data: { | |
levelProperties: LevelProperties; | |
channel?: Channel; | |
initialSources?: ProjectSources; | |
}, | |
aborted: boolean, | |
dispatch: ThunkDispatch<unknown, unknown, AnyAction> | |
) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: maybe rename to updateAndSaveAiCustomizations or just saveAiCustomizations?
+1 on saveAiCustomizations
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated here -- went with saveAiCustomization
for the thunk that is exported, and saveAiCustomizationShared
for the shared code since it aligns exactly with what happens when you use saveAiCustomization
. LMK if that works for y'all!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems like saveAiCustomizationShared
is similar to an internal or private function. Is there a convention we could establish for these 'shared' functions that are used internally? 'Shared 'seems to have multiple meanings so may be a little unclear to a reader.
const selectModelCardInfo = (state: {aichat: AichatState}) => | ||
state.aichat.currentAiCustomizations.modelCardInfo; | ||
|
||
export const selectHasFilledOutModelCard = createSelector( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TIL about createSelector
!
apps/src/aichat/redux/aichatRedux.ts
Outdated
}; | ||
|
||
// Selectors | ||
const selectModelCardInfo = (state: {aichat: AichatState}) => |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this need to be a separate selector or could this just be declared inline in selectHasFilledOutModelCard
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think inline is fine, I was inspired by Molly's work here -- updated!
code-dot-org/apps/src/javalab/redux/editorRedux.ts
Lines 272 to 287 in d27cc42
const selectSources = (state: {javalabEditor: JavalabEditorState}) => | |
state.javalabEditor.sources; | |
export const getSources = createSelector(selectSources, sources => { | |
const result: EditorFilesMap = {}; | |
for (const key in sources) { | |
if (!sources[key].isValidation) { | |
result[key] = { | |
text: sources[key].text, | |
isVisible: sources[key].isVisible, | |
tabOrder: sources[key].tabOrder, | |
}; | |
} | |
} | |
return result; | |
}); |
return property === 'botName' ? 'input' : 'textarea'; | ||
}; | ||
|
||
const renderPublishOkNotification = () => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't exactly know what the performance implications are, but thoughts on just making these components instead of functions that create components every time the parent component is rendered?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hmm good q, I think I just had these returning HTML originally before refactoring :) I think I moved them out of the render function just for readability (and I just added some more logic to cover an edge case Alice identified here). Is there a difference between this and an inline ternary that returns a component on each render? Isn't the component created each time in that case as well, or is React smarter in that case and keeps them around or something?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I think the key difference is using functions to render vs values? I don't think it matters a whole lot since these are small, but because they're static components, I was thinking we could benefit from memoizing them. I think we can either just make them static values if they're outside the main component (const publishOkNotification = <PublishStatus ... props />
), or use useMemo
inside the component (const publishOkNotification = useMemo(renderPublishOkNotification))
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mentioned at #57949 (comment), but if a separate component, could use in other customization panels like 'Retrieval' (if retrieval context added/deleted but not updated yet).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is already ready as a reusable component, just would need to be renamed from PublishStatus
.
d'oh re: static value / no args, updated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome - thanks!
apps/src/aichat/redux/aichatRedux.ts
Outdated
export const publishModel = createAsyncThunk( | ||
'aichat/publishModelCard', | ||
async (_, thunkAPI) => { | ||
thunkAPI.dispatch(setHasPublished(true)); | ||
await updateAiCustomizationShared(thunkAPI); | ||
thunkAPI.dispatch(setViewMode(ViewMode.PRESENTATION)); | ||
} | ||
); | ||
|
||
export const saveModelCard = createAsyncThunk( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nice refactoring! mind dropping some comments around how the code is organized between these various thunks and the shared function?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool refactoring - learned a lot here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Left a question about if the model card isreadonly
but I think this is a solid checkpoint since I know you're planning a fast follow-up.
Learned a lot and thanks for the iteration and discussion!
apps/src/aichat/redux/aichatRedux.ts
Outdated
); | ||
|
||
// eslint-disable-next-line @typescript-eslint/no-explicit-any | ||
const updateAiCustomizationShared = async (thunkAPI: any) => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: maybe rename to updateAndSaveAiCustomizations or just saveAiCustomizations?
+1 on saveAiCustomizations
const onPublish = useCallback(() => { | ||
dispatch(updateAiCustomization()); | ||
dispatch(setHasPublished(true)); | ||
dispatch(setViewMode(ViewMode.PRESENTATION)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That would be great as a follow-up and +1 on using Denys's new Tabs component - thanks!
const onPublish = useCallback(() => { | ||
dispatch(updateAiCustomization()); | ||
dispatch(setHasPublished(true)); | ||
dispatch(setViewMode(ViewMode.PRESENTATION)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also related to comment below, that 'User View' remains displayed?
Not sure I'm following this part, happy to chat offline if easier!
Ah sorry. I think once we save hasPublished
to sources, this will not be an issue when a user moves from level to level. I think I was referring to how when a model card has been published, the toggle remain visible.
apps/src/aichat/redux/aichatRedux.ts
Outdated
export const publishModel = createAsyncThunk( | ||
'aichat/publishModelCard', | ||
async (_, thunkAPI) => { | ||
thunkAPI.dispatch(setHasPublished(true)); | ||
await updateAiCustomizationShared(thunkAPI); | ||
thunkAPI.dispatch(setViewMode(ViewMode.PRESENTATION)); | ||
} | ||
); | ||
|
||
export const saveModelCard = createAsyncThunk( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool refactoring - learned a lot here.
!levelAichatSettings?.hidePresentationPanel && | ||
(hasPublished || | ||
(levelAichatSettings?.visibilities && | ||
isDisabled(levelAichatSettings.visibilities.modelCardInfo))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if this is P0 - If levelbuilders mark visibility of modelCardInfo
as readonly
, then it is assumed that all of the fields in model card should be filled out. Maybe worth checking with Dan on this assumption or do levelbuilders need a reminder about this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this is a P0, we can add a levelbuilder-side check for this as well
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I already did it, so I'll leave this for now (I think its a sensible default). We can clarify the finer points with levelbuilders / validate down the road
apps/src/aichat/redux/aichatRedux.ts
Outdated
}; | ||
|
||
// THUNKS | ||
|
||
// This thunk saves a student's AI customizations using the Project Manager (ie, to S3 typically), | ||
// then does a comparison between the previous and current saved customizations in order to | ||
// output a message to the chat window with the list of customizations that were updated. | ||
export const updateAiCustomization = createAsyncThunk( | ||
'aichat/updateAiCustomization', | ||
export const saveAiCustomization = createAsyncThunk( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
very minor / optional - name this updateAiCustomizations
to match "Update" button and keep the shared function just saveAiCustomizations
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Commented on this already but I think this would be more clear as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok you guys win, updated
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
😁 Thanks for iterating on this!
!levelAichatSettings?.hidePresentationPanel && | ||
(hasPublished || | ||
(levelAichatSettings?.visibilities && | ||
isDisabled(levelAichatSettings.visibilities.modelCardInfo))) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this is a P0, we can add a levelbuilder-side check for this as well
Warning!!
The AP CSP Create Performance Task is in progress. The most critical dates are from April 3 - April 30, 2024. Please consider any risk introduced by this PR that could affect our students taking AP CSP. Code.org students taking AP CSP primarily use App Lab for their Create Task, however a small percent use Game Lab. Carefully consider whether your change has any risk of alterering, changing, or breaking anything in these two labs. Even small changes, such as a different button color, are considered significant during this time period. Reach out to the Student Learning team or Curriculum team for more details.
Description
A few related updates related to splitting the "Publish" button in the "Publish" tab into separate "Publish" and "Save" buttons:
hasPublished
state), and show the toggle at that point.publish-and-save-buttons.mov
Testing story
Tested manually -- see video above. Also tested that changing the hide presentation view setting in levelbuilder resulted in the "Publish" tab being hidden.
Follow-up work
A couple follow-ups I can think of:
hasPublished
not being saved to S3 (ie, state does not persist between page loads). We need to update this, but I couldn't quite figure out whether to add it as part of theAiCustomizations
type, or store it separately. In any case, it felt like we could do it as a fast follow.