Skip to content

Commit

Permalink
[editor] Only Persist Changed Model Settings (#551)
Browse files Browse the repository at this point in the history
# Only Persist Changed Model Settings

Fix a bug in the settings renderer copied from workbooks -- currently,
just opening the settings tab and saving the workbook will persist a
model settings object for the prompt. The cause of this is the use of
the `useEffect` for handling changes -- `useEffect` will always run once
on mount of the component so it was always persisting the empty settings
on mount.

To fix, just check that the value has actually changed before persisting
it to the config.

## Testing:
- Open settings & save config without touching anything. See the config
has no changes
- Change system prompt and save, ensure it's persisted to the config
---
Stack created with [Sapling](https://sapling-scm.com). Best reviewed
with
[ReviewStack](https://reviewstack.dev/lastmile-ai/aiconfig/pull/551).
* #552
* __->__ #551
  • Loading branch information
rholinshead authored Dec 20, 2023
2 parents 7b2857d + 6d34814 commit 6b76233
Show file tree
Hide file tree
Showing 2 changed files with 6 additions and 2 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import {
import { useState, useCallback, useEffect, useMemo, memo, useRef } from "react";
import { uniqueId } from "lodash";
import { IconHelp, IconPlus, IconTrash } from "@tabler/icons-react";
import { usePrevious } from "@mantine/hooks";

type Props = {
propertyName: string;
Expand Down Expand Up @@ -61,7 +62,11 @@ export default function SettingsPropertyRenderer({

let propertyControl;

const prevValue = usePrevious(propertyValue);
useEffect(() => {
if (prevValue === propertyValue) {
return;
}
if (propertyName != null && propertyName.trim() !== "") {
setValue((oldValue: any) => {
return {
Expand All @@ -72,7 +77,7 @@ export default function SettingsPropertyRenderer({
} else {
setValue(propertyValue);
}
}, [propertyName, propertyValue, setValue]);
}, [prevValue, propertyName, propertyValue, setValue]);

// Used in the case the property is an array
// TODO: Should initialize with values from settings if available
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ export const OpenAIChatModelParserPromptSchema: PromptSchema = {
type: "integer",
maximum: 4096,
minimum: 16,
default: 4096,
},
n: {
type: "integer",
Expand Down

0 comments on commit 6b76233

Please sign in to comment.