Skip to content
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

Fix saving model between chats #2843

Merged
merged 4 commits into from Apr 25, 2023
Merged

Fix saving model between chats #2843

merged 4 commits into from Apr 25, 2023

Conversation

AbdBarho
Copy link
Collaborator

@AbdBarho AbdBarho commented Apr 22, 2023

You can see this bug on dev or staging if you try changing the model couple of times.

Somewhere within the mess of server side / client side rendering the model config gets overwritten.

}
}, [config, formState.isDirty]);
setConfigCache(config);
}, [config]);
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when we change presets, we use the function reset, which sets the isDirty to false, so we are not saving.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The isDirty check is require to avoid update on the first render which use the first model config from serverside, not the one from localstorage

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should pass keepDirty option when update the preset:

reset({ ...config, model_config_name: selectedModel }, {
     keepDirty: true,
});

Copy link
Collaborator Author

@AbdBarho AbdBarho Apr 24, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I refactored it, I added keep dirty like you said, and switched to using Controller instead of register and it works now

@AbdBarho AbdBarho marked this pull request as ready for review April 22, 2023 15:37
@notmd
Copy link
Collaborator

notmd commented Apr 25, 2023

@AbdBarho I just the pushed the change with correct fix by using getValues. Look like useWatch only update the value on the next render. The docs also recommend to use getValue to access latest value immediately https://react-hook-form.com/api/usewatch/. I gonna merge this since Dragan requested this, so he can work on the plugin feature.

@notmd notmd enabled auto-merge (squash) April 25, 2023 08:16
@notmd notmd merged commit dcf5456 into main Apr 25, 2023
4 checks passed
@notmd notmd deleted the save-config branch April 25, 2023 08:16
grgau pushed a commit to grgau/Open-Assistant that referenced this pull request May 8, 2023
You can see this bug on dev or staging if you try changing the model
couple of times.

Somewhere within the mess of server side / client side rendering the
model config gets overwritten.

---------

Co-authored-by: notmd <notmd1811@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants