From 0979b4300ae04959ca499529e97ed9d6e55f5658 Mon Sep 17 00:00:00 2001 From: James Melvin Date: Fri, 13 Jan 2023 18:58:42 +0530 Subject: [PATCH 01/11] fix: non-empty text before submitting on create task --- website/src/components/Tasks/CreateTask.tsx | 11 ++++++++--- website/src/components/Tasks/Task.tsx | 4 ++++ website/src/types/TaskReplyState.ts | 6 +++++- 3 files changed, 17 insertions(+), 4 deletions(-) diff --git a/website/src/components/Tasks/CreateTask.tsx b/website/src/components/Tasks/CreateTask.tsx index b66eebb0..0bc3c7de 100644 --- a/website/src/components/Tasks/CreateTask.tsx +++ b/website/src/components/Tasks/CreateTask.tsx @@ -11,11 +11,16 @@ export const CreateTask = ({ task, taskType, onReplyChanged }: TaskSurveyProps<{ const labelColor = useColorModeValue("gray.600", "gray.400"); const [inputText, setInputText] = useState(""); - const textChangeHandler = (event: React.ChangeEvent) => { const text = event.target.value; - onReplyChanged({ content: { text }, state: "VALID" }); - setInputText(text); + const isTextBlank = (!text || /^\s*$/.test(text))?true:false; + if(!isTextBlank){ + onReplyChanged({ content: { text }, state: "VALID" }); + setInputText(text); + }else{ + onReplyChanged({content: { text }, state: "INVALID"}); + setInputText(""); + } }; return ( diff --git a/website/src/components/Tasks/Task.tsx b/website/src/components/Tasks/Task.tsx index 1ccef66c..f5bac231 100644 --- a/website/src/components/Tasks/Task.tsx +++ b/website/src/components/Tasks/Task.tsx @@ -50,7 +50,10 @@ export const Task = ({ frontendId, task, trigger, mutate }) => { if (taskStatus !== "DEFAULT") setTaskStatus("DEFAULT"); } else if (state.state === "VALID") { if (taskStatus !== "SUBMITABLE") setTaskStatus("SUBMITABLE"); + } else if (state.state == "INVALID") { + setTaskStatus("NOT_SUBMITTABLE"); } + console.log(taskStatus); }).current; const submitResponse = () => { @@ -94,6 +97,7 @@ export const Task = ({ frontendId, task, trigger, mutate }) => { onSubmit={submitResponse} onSkip={rejectTask} onNextTask={mutate} + key={taskStatus} /> { content: T; state: "DEFAULT"; } +export interface TaskReplyInValid { + content: T; + state: "INVALID"; +} -export type TaskReplyState = TaskReplyValid | TaskReplyDefault; +export type TaskReplyState = TaskReplyValid | TaskReplyDefault | TaskReplyInValid; From 8067e1a879ef5f29353889e6074ac6943ff9ad8a Mon Sep 17 00:00:00 2001 From: James Melvin Date: Fri, 13 Jan 2023 19:04:51 +0530 Subject: [PATCH 02/11] fix: pre-commit changes --- website/src/components/Tasks/CreateTask.tsx | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/website/src/components/Tasks/CreateTask.tsx b/website/src/components/Tasks/CreateTask.tsx index 0bc3c7de..448258e9 100644 --- a/website/src/components/Tasks/CreateTask.tsx +++ b/website/src/components/Tasks/CreateTask.tsx @@ -13,12 +13,12 @@ export const CreateTask = ({ task, taskType, onReplyChanged }: TaskSurveyProps<{ const [inputText, setInputText] = useState(""); const textChangeHandler = (event: React.ChangeEvent) => { const text = event.target.value; - const isTextBlank = (!text || /^\s*$/.test(text))?true:false; - if(!isTextBlank){ + const isTextBlank = !text || /^\s*$/.test(text) ? true : false; + if (!isTextBlank) { onReplyChanged({ content: { text }, state: "VALID" }); setInputText(text); - }else{ - onReplyChanged({content: { text }, state: "INVALID"}); + } else { + onReplyChanged({ content: { text }, state: "INVALID" }); setInputText(""); } }; From b24b47d8bd3c839a13a50bdffdd5cab284254b38 Mon Sep 17 00:00:00 2001 From: James Melvin Date: Fri, 13 Jan 2023 19:10:50 +0530 Subject: [PATCH 03/11] fix: merge conflict --- website/src/components/Tasks/Task.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/website/src/components/Tasks/Task.tsx b/website/src/components/Tasks/Task.tsx index f5bac231..dc94c2d9 100644 --- a/website/src/components/Tasks/Task.tsx +++ b/website/src/components/Tasks/Task.tsx @@ -97,7 +97,6 @@ export const Task = ({ frontendId, task, trigger, mutate }) => { onSubmit={submitResponse} onSkip={rejectTask} onNextTask={mutate} - key={taskStatus} /> Date: Fri, 13 Jan 2023 13:57:55 +0000 Subject: [PATCH 04/11] Set some message trees into ranking state after seed-data insertion --- backend/main.py | 13 ++++++++++-- backend/oasst_backend/prompt_repository.py | 14 +++++++------ .../realistic/realistic_seed_data.json | 21 ++++++++++++------- 3 files changed, 33 insertions(+), 15 deletions(-) diff --git a/backend/main.py b/backend/main.py index c13e56df..76b0a35b 100644 --- a/backend/main.py +++ b/backend/main.py @@ -107,6 +107,7 @@ if settings.DEBUG_USE_SEED_DATA: parent_message_id: Optional[str] text: str role: str + tree_state: Optional[message_tree_state.State] try: logger.info("Seed data check began") @@ -157,10 +158,17 @@ if settings.DEBUG_USE_SEED_DATA: ) tr.bind_frontend_message_id(task.id, msg.task_message_id) message = pr.store_text_reply( - msg.text, msg.task_message_id, msg.user_message_id, review_count=5, review_result=True + msg.text, + msg.task_message_id, + msg.user_message_id, + review_count=5, + review_result=True, + check_tree_state=False, ) if message.parent_id is None: - tm._insert_default_state(root_message_id=message.id, state=message_tree_state.State.GROWING) + tm._insert_default_state( + root_message_id=message.id, state=msg.tree_state or message_tree_state.State.GROWING + ) db.commit() logger.info( @@ -168,6 +176,7 @@ if settings.DEBUG_USE_SEED_DATA: ) else: logger.debug(f"seed data task found: {task.id}") + logger.info("Seed data check completed") except Exception: diff --git a/backend/oasst_backend/prompt_repository.py b/backend/oasst_backend/prompt_repository.py index 5bae3b18..cb6e70f7 100644 --- a/backend/oasst_backend/prompt_repository.py +++ b/backend/oasst_backend/prompt_repository.py @@ -140,6 +140,7 @@ class PromptRepository: user_frontend_message_id: str, review_count: int = 0, review_result: bool = False, + check_tree_state: bool = True, ) -> Message: validate_frontend_message_id(frontend_message_id) validate_frontend_message_id(user_frontend_message_id) @@ -155,12 +156,13 @@ class PromptRepository: parent_message = self.fetch_message(task.parent_message_id) # check tree state - ts = self.fetch_tree_state(parent_message.message_tree_id) - if not ts.active or ts.state != message_tree_state.State.GROWING: - raise OasstError( - "Message insertion failed. Message tree is no longer in 'growing' state.", - OasstErrorCode.TREE_NOT_IN_GROWING_STATE, - ) + if check_tree_state: + ts = self.fetch_tree_state(parent_message.message_tree_id) + if not ts.active or ts.state != message_tree_state.State.GROWING: + raise OasstError( + "Message insertion failed. Message tree is no longer in 'growing' state.", + OasstErrorCode.TREE_NOT_IN_GROWING_STATE, + ) parent_message.message_tree_id parent_message.children_count += 1 diff --git a/backend/test_data/realistic/realistic_seed_data.json b/backend/test_data/realistic/realistic_seed_data.json index be2a17e9..ce8e7ff3 100644 --- a/backend/test_data/realistic/realistic_seed_data.json +++ b/backend/test_data/realistic/realistic_seed_data.json @@ -228,7 +228,8 @@ "user_message_id": "rrbopvrb", "parent_message_id": null, "text": " How can I feed squirrels and make more come to my home?", - "role": "prompter" + "role": "prompter", + "tree_state": "ranking" }, { "task_message_id": "92w6z5iw", @@ -1054,7 +1055,8 @@ "user_message_id": "3pvmby5u", "parent_message_id": null, "text": " My 35 year old son is having pain from his jaw, neck and shoulder blade", - "role": "prompter" + "role": "prompter", + "tree_state": "ranking" }, { "task_message_id": "v891mplm", @@ -1418,7 +1420,8 @@ "user_message_id": "4j9k8rqq", "parent_message_id": null, "text": " I have been gaining weight and want to start exercising. How should I begin?", - "role": "prompter" + "role": "prompter", + "tree_state": "ranking" }, { "task_message_id": "kfg4s8bg", @@ -1600,7 +1603,8 @@ "user_message_id": "vgtyn75k", "parent_message_id": null, "text": " What is macular degeneration?", - "role": "prompter" + "role": "prompter", + "tree_state": "ranking" }, { "task_message_id": "zusgcbfj", @@ -2041,7 +2045,8 @@ "user_message_id": "vtaes87s", "parent_message_id": null, "text": " I keep biting the inside of my cheek when I'm sleeping. What can I do to prevent this in the future?", - "role": "prompter" + "role": "prompter", + "tree_state": "ranking" }, { "task_message_id": "zxumt49x", @@ -2636,7 +2641,8 @@ "user_message_id": "v14u082o", "parent_message_id": null, "text": " Any tips on how to revise something you wrote?", - "role": "prompter" + "role": "prompter", + "tree_state": "ranking" }, { "task_message_id": "bcw4led5", @@ -2986,7 +2992,8 @@ "user_message_id": "di54e1nb", "parent_message_id": null, "text": " I am going to Singapore on business. Can you tell me what you know about Singapore?", - "role": "prompter" + "role": "prompter", + "tree_state": "ranking" }, { "task_message_id": "z79vpkaj", From 786553e00abc35fcb29dbdb48e8d6e6435f165dc Mon Sep 17 00:00:00 2001 From: James Melvin Date: Fri, 13 Jan 2023 19:59:13 +0530 Subject: [PATCH 05/11] fix: removed console log --- website/src/components/Tasks/Task.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/website/src/components/Tasks/Task.tsx b/website/src/components/Tasks/Task.tsx index dc94c2d9..e97b3d4d 100644 --- a/website/src/components/Tasks/Task.tsx +++ b/website/src/components/Tasks/Task.tsx @@ -53,7 +53,6 @@ export const Task = ({ frontendId, task, trigger, mutate }) => { } else if (state.state == "INVALID") { setTaskStatus("NOT_SUBMITTABLE"); } - console.log(taskStatus); }).current; const submitResponse = () => { From eb43c8b4f829cbcedecf161738063fe23386c831 Mon Sep 17 00:00:00 2001 From: Adrian Cowan Date: Sat, 14 Jan 2023 01:25:46 +1100 Subject: [PATCH 06/11] website: Add a review step before submitting to hint to the user that they should reread and check what they have written. Also show numbers for ranked items rather than drag handles in the review step as the items can't be moved in that step. --- website/cypress/e2e/tasks/random.cy.ts | 4 ++ .../components/Sortable/Sortable.stories.tsx | 6 +- website/src/components/Sortable/Sortable.tsx | 5 +- .../src/components/Sortable/SortableItem.tsx | 16 +++-- .../src/components/Survey/TaskControls.tsx | 41 ++++++++--- website/src/components/Tasks/CreateTask.tsx | 10 ++- website/src/components/Tasks/EvaluateTask.tsx | 15 +++- website/src/components/Tasks/LabelTask.tsx | 14 ++-- website/src/components/Tasks/Task.tsx | 68 +++++++++++++------ website/src/components/Tasks/TaskTypes.tsx | 6 +- .../src/components/Tasks/UnchangedWarning.tsx | 5 +- 11 files changed, 134 insertions(+), 56 deletions(-) diff --git a/website/cypress/e2e/tasks/random.cy.ts b/website/cypress/e2e/tasks/random.cy.ts index 337ed331..2ab3bf40 100644 --- a/website/cypress/e2e/tasks/random.cy.ts +++ b/website/cypress/e2e/tasks/random.cy.ts @@ -15,6 +15,8 @@ describe("handles random tasks", () => { cy.log("reply", reply); cy.get('[data-cy="reply"]').type(reply); + cy.get('[data-cy="review"]').click(); + cy.get('[data-cy="submit"]').click(); cy.get('[data-cy="task-id]"').should((taskIdElement) => { @@ -38,6 +40,8 @@ describe("handles random tasks", () => { .wait(100) .type("{enter}"); + cy.get('[data-cy="review"]').click(); + cy.get('[data-cy="submit"]').click(); cy.get('[data-cy="task-id"]').should((taskIdElement) => { diff --git a/website/src/components/Sortable/Sortable.stories.tsx b/website/src/components/Sortable/Sortable.stories.tsx index 93c6510b..db588cb4 100644 --- a/website/src/components/Sortable/Sortable.stories.tsx +++ b/website/src/components/Sortable/Sortable.stories.tsx @@ -8,8 +8,8 @@ export default { component: Sortable, }; -const Template = ({ items, isDisabled }) => { - return ; +const Template = ({ items, isEditable, isDisabled }) => { + return ; }; export const Default = Template.bind({}); @@ -19,6 +19,7 @@ Default.args = { "euirdteunvglfe23908230892309832098 AAAAAAAA", "Sorry, my cat sat on my keyboard. Can you print a cat in ASCII art?", ], + isEditable: true, isDisabled: false, }; @@ -29,5 +30,6 @@ LongText.args = { "Assistant: Yes, I think they can be helpful when the child misbehaves, but they should be used with a little bit of compassion and understanding that it\u2019s not the natural state of things to have an adult yelling at them. Time outs are also often used without letting the child know how they\u2019re getting out of the time out, which can make it feel arbitrary or like a punishment, rather than a consequence for something they did. It\u2019s really easy for adults to do this kind of thing unconsciously. It\u2019s easy to get caught up in the notion that \u201cThey\u2019re in time out, and that\u2019s the end of it!\u201d but kids can be pretty imaginative, and they can use their own creativity to make their way out of time outs. A compassionate time out ends when the child shows a sign of understanding what they\u2019ve done wrong, and are ready to begin again. That way the child knows they\u2019re learning, and that the parent is seeing them as an intelligent person, even if they sometimes mess up. You can still use the other techniques you were using to be tough when necessary, but using a compassionate approach will let you use them without actually using them!", "Assistant: No. The USA was founded by a Puritan group of Protestants, but it didn\u2019t adopt the religion of the Puritans until much later, and it was always a secular state. The Puritans observed the Sabbath on Sunday, and the Puritans only had a small influence in the early history of the USA. It\u2019s difficult to trace the origins of closing stores on Sunday, but one early and short-lived attempt at forcing the Sabbath on people in the 1800s was motivated by the Protestant ideal that people should spend Sunday focusing on spiritual activities. By the mid-1800s, when the Sunday closing law was made, there was not a lot of pressure from that standpoint, but the church had begun to advocate for Sunday closing laws as a way of counteracting the negative effects of industrialization on the day of rest. Even after that shift, closing stores on Sunday was not always possible, since the religious Sunday was not always chosen for observance. And as industrialization accelerated and mechanization made it possible to operate stores on Sunday, the law was not enforced as much as people liked. The day of rest was also being violated by stores that stayed open all day on Sunday, so closing stores on Sundays became an effort to protect the Sabbath for all citizens.", ], + isEditable: true, isDisabled: false, }; diff --git a/website/src/components/Sortable/Sortable.tsx b/website/src/components/Sortable/Sortable.tsx index 48c2916b..596b341d 100644 --- a/website/src/components/Sortable/Sortable.tsx +++ b/website/src/components/Sortable/Sortable.tsx @@ -24,6 +24,7 @@ import { SortableItem } from "./SortableItem"; export interface SortableProps { items: ReactNode[]; onChange?: (newSortedIndices: number[]) => void; + isEditable: boolean; isDisabled?: boolean; className?: string; } @@ -64,8 +65,8 @@ export const Sortable = (props: SortableProps) => { > - {itemsWithIds.map(({ id, item }) => ( - + {itemsWithIds.map(({ id, item }, index) => ( + ))} diff --git a/website/src/components/Sortable/SortableItem.tsx b/website/src/components/Sortable/SortableItem.tsx index 811cef02..56722694 100644 --- a/website/src/components/Sortable/SortableItem.tsx +++ b/website/src/components/Sortable/SortableItem.tsx @@ -4,12 +4,18 @@ import { CSS } from "@dnd-kit/utilities"; import { PropsWithChildren, useState } from "react"; import { RxDragHandleDots2 } from "react-icons/rx"; -export const SortableItem = ({ children, id, isDisabled }: PropsWithChildren<{ id: number; isDisabled: boolean }>) => { +export const SortableItem = ({ + children, + id, + index, + isEditable, + isDisabled, +}: PropsWithChildren<{ id: number; index: number; isEditable: boolean; isDisabled: boolean }>) => { const backgroundColor = useColorModeValue("gray.700", "gray.500"); const disabledBackgroundColor = useColorModeValue("gray.400", "gray.700"); const textColor = useColorModeValue("white", "white"); - const { attributes, listeners, setNodeRef, transform, transition } = useSortable({ id, disabled: isDisabled }); + const { attributes, listeners, setNodeRef, transform, transition } = useSortable({ id, disabled: !isEditable }); const style = { transform: CSS.Translate.toString(transform), @@ -27,7 +33,7 @@ export const SortableItem = ({ children, id, isDisabled }: PropsWithChildren<{ i borderRadius="lg" p="4" color={textColor} - cursor={isDisabled ? "auto" : grabbing ? "grabbing" : "grab"} + cursor={isEditable ? (grabbing ? "grabbing" : "grab") : "auto"} onMouseDown={() => { setGrabbing(true); }} @@ -38,9 +44,7 @@ export const SortableItem = ({ children, id, isDisabled }: PropsWithChildren<{ i style={style} shadow="base" > - - - + {isEditable ? : `${index + 1}.`} {children} ); diff --git a/website/src/components/Survey/TaskControls.tsx b/website/src/components/Survey/TaskControls.tsx index 81b2c775..c45b5ce0 100644 --- a/website/src/components/Survey/TaskControls.tsx +++ b/website/src/components/Survey/TaskControls.tsx @@ -1,4 +1,5 @@ -import { Box, Flex, useColorModeValue } from "@chakra-ui/react"; +import { Box, Flex, IconButton, Tooltip, useColorModeValue } from "@chakra-ui/react"; +import { FiEdit2 } from "react-icons/fi"; import { SkipButton } from "src/components/Buttons/Skip"; import { SubmitButton } from "src/components/Buttons/Submit"; import { TaskInfo } from "src/components/TaskInfo/TaskInfo"; @@ -10,6 +11,8 @@ export interface TaskControlsProps { task: any; className?: string; taskStatus: TaskStatus; + onEdit: () => void; + onReview: () => void; onSubmit: () => void; onSkip: (reason: string) => void; } @@ -30,15 +33,33 @@ export const TaskControls = (props: TaskControlsProps) => { > - - - Submit - + {props.taskStatus === "REVIEW" || props.taskStatus === "SUBMITTED" ? ( + <> + + } /> + + + Submit + + + ) : ( + <> + + + Review + + + )} ); diff --git a/website/src/components/Tasks/CreateTask.tsx b/website/src/components/Tasks/CreateTask.tsx index 652a867c..27273acf 100644 --- a/website/src/components/Tasks/CreateTask.tsx +++ b/website/src/components/Tasks/CreateTask.tsx @@ -5,7 +5,13 @@ import { TrackedTextarea } from "src/components/Survey/TrackedTextarea"; import { TwoColumnsWithCards } from "src/components/Survey/TwoColumnsWithCards"; import { TaskSurveyProps } from "src/components/Tasks/Task"; -export const CreateTask = ({ task, taskType, isDisabled, onReplyChanged }: TaskSurveyProps<{ text: string }>) => { +export const CreateTask = ({ + task, + taskType, + isEditable, + isDisabled, + onReplyChanged, +}: TaskSurveyProps<{ text: string }>) => { const cardColor = useColorModeValue("gray.100", "gray.700"); const titleColor = useColorModeValue("gray.800", "gray.300"); const labelColor = useColorModeValue("gray.600", "gray.400"); @@ -50,7 +56,7 @@ export const CreateTask = ({ task, taskType, isDisabled, onReplyChanged }: TaskS text={inputText} onTextChange={textChangeHandler} thresholds={{ low: 20, medium: 40, goal: 50 }} - textareaProps={{ placeholder: "Write your prompt here...", isDisabled }} + textareaProps={{ placeholder: "Write your prompt here...", isDisabled, isReadOnly: !isEditable }} /> diff --git a/website/src/components/Tasks/EvaluateTask.tsx b/website/src/components/Tasks/EvaluateTask.tsx index 87ee86ea..7e99f490 100644 --- a/website/src/components/Tasks/EvaluateTask.tsx +++ b/website/src/components/Tasks/EvaluateTask.tsx @@ -5,7 +5,12 @@ import { Sortable } from "src/components/Sortable/Sortable"; import { SurveyCard } from "src/components/Survey/SurveyCard"; import { TaskSurveyProps } from "src/components/Tasks/Task"; -export const EvaluateTask = ({ task, isDisabled, onReplyChanged }: TaskSurveyProps<{ ranking: number[] }>) => { +export const EvaluateTask = ({ + task, + isEditable, + isDisabled, + onReplyChanged, +}: TaskSurveyProps<{ ranking: number[] }>) => { const cardColor = useColorModeValue("gray.100", "gray.700"); const titleColor = useColorModeValue("gray.800", "gray.300"); const labelColor = useColorModeValue("gray.600", "gray.400"); @@ -46,7 +51,13 @@ export const EvaluateTask = ({ task, isDisabled, onReplyChanged }: TaskSurveyPro - + diff --git a/website/src/components/Tasks/LabelTask.tsx b/website/src/components/Tasks/LabelTask.tsx index ea56ab34..fe508600 100644 --- a/website/src/components/Tasks/LabelTask.tsx +++ b/website/src/components/Tasks/LabelTask.tsx @@ -12,7 +12,7 @@ export const LabelTask = ({ task, taskType, onReplyChanged, - isDisabled, + isEditable, }: TaskSurveyProps<{ text: string; labels: { [k: string]: number }; message_id: string }>) => { const valid_labels = task.valid_labels; const [sliderValues, setSliderValues] = useState(new Array(valid_labels.length).fill(0)); @@ -65,7 +65,7 @@ export const LabelTask = ({ )} - + ); @@ -75,10 +75,10 @@ export const LabelTask = ({ interface LabelSliderGroupProps { labelIDs: Array; onChange: (sliderValues: number[]) => unknown; - isDisabled?: boolean; + isEditable: boolean; } -export const LabelSliderGroup = ({ labelIDs, onChange, isDisabled }: LabelSliderGroupProps) => { +export const LabelSliderGroup = ({ labelIDs, onChange, isEditable }: LabelSliderGroupProps) => { const [sliderValues, setSliderValues] = useState(Array.from({ length: labelIDs.length }).map(() => 0)); return ( @@ -94,7 +94,7 @@ export const LabelSliderGroup = ({ labelIDs, onChange, isDisabled }: LabelSlider onChange(sliderValues); setSliderValues(newState); }} - isDisabled={isDisabled} + isEditable={isEditable} /> ))} @@ -105,7 +105,7 @@ function CheckboxSliderItem(props: { labelId: string; sliderValue: number; sliderHandler: (newVal: number) => unknown; - isDisabled: boolean; + isEditable: boolean; }) { const id = useId(); const { colorMode } = useColorMode(); @@ -118,7 +118,7 @@ function CheckboxSliderItem(props: { {/* TODO: display real text instead of just the id */} {props.labelId} - props.sliderHandler(val / 100)}> + props.sliderHandler(val / 100)}> diff --git a/website/src/components/Tasks/Task.tsx b/website/src/components/Tasks/Task.tsx index e02f5922..f9b5e50d 100644 --- a/website/src/components/Tasks/Task.tsx +++ b/website/src/components/Tasks/Task.tsx @@ -10,13 +10,14 @@ import { TaskContent } from "src/types/Task"; import { TaskReplyState } from "src/types/TaskReplyState"; import useSWRMutation from "swr/mutation"; -export type TaskStatus = "NOT_SUBMITTABLE" | "DEFAULT" | "SUBMITABLE" | "SUBMITTED"; +export type TaskStatus = "NOT_SUBMITTABLE" | "DEFAULT" | "VALID" | "REVIEW" | "SUBMITTED"; export interface TaskSurveyProps { // we need a task type // eslint-disable-next-line @typescript-eslint/no-explicit-any task: any; taskType: TaskInfo; + isEditable: boolean; isDisabled?: boolean; onReplyChanged: (state: TaskReplyState) => void; } @@ -50,20 +51,38 @@ export const Task = ({ frontendId, task, trigger, mutate }) => { } else if (state.state === "DEFAULT") { if (taskStatus !== "DEFAULT") setTaskStatus("DEFAULT"); } else if (state.state === "VALID") { - if (taskStatus !== "SUBMITABLE") setTaskStatus("SUBMITABLE"); - } else if (state.state == "INVALID") { + if (taskStatus !== "VALID") setTaskStatus("VALID"); + } else if (state.state === "INVALID") { setTaskStatus("NOT_SUBMITTABLE"); } }).current; - const submitResponse = () => { + const reviewResponse = () => { switch (taskStatus) { - case "NOT_SUBMITTABLE": - return; case "DEFAULT": setShowUnchangedWarning(true); break; - case "SUBMITABLE": { + case "VALID": + setTaskStatus("REVIEW"); + break; + default: + return; + } + }; + + const editResponse = () => { + switch (taskStatus) { + case "REVIEW": + setTaskStatus("VALID"); + break; + default: + return; + } + }; + + const submitResponse = () => { + switch (taskStatus) { + case "REVIEW": { trigger({ id: frontendId, update_type: taskType.update_type, @@ -72,11 +91,14 @@ export const Task = ({ frontendId, task, trigger, mutate }) => { setTaskStatus("SUBMITTED"); break; } - case "SUBMITTED": + default: return; } }; + const edit_mode = taskStatus === "NOT_SUBMITTABLE" || taskStatus === "DEFAULT" || taskStatus === "VALID"; + const submitted = taskStatus === "SUBMITTED"; + function taskTypeComponent() { switch (taskType.category) { case TaskCategory.Create: @@ -85,7 +107,8 @@ export const Task = ({ frontendId, task, trigger, mutate }) => { key={task.id} task={task} taskType={taskType} - isDisabled={taskStatus === "SUBMITTED"} + isEditable={edit_mode} + isDisabled={submitted} onReplyChanged={onReplyChanged} /> ); @@ -95,7 +118,8 @@ export const Task = ({ frontendId, task, trigger, mutate }) => { key={task.id} task={task} taskType={taskType} - isDisabled={taskStatus === "SUBMITTED"} + isEditable={edit_mode} + isDisabled={submitted} onReplyChanged={onReplyChanged} /> ); @@ -105,7 +129,8 @@ export const Task = ({ frontendId, task, trigger, mutate }) => { key={task.id} task={task} taskType={taskType} - isDisabled={taskStatus === "SUBMITTED"} + isEditable={edit_mode} + isDisabled={submitted} onReplyChanged={onReplyChanged} /> ); @@ -115,20 +140,23 @@ export const Task = ({ frontendId, task, trigger, mutate }) => { return (
{taskTypeComponent()} - + setShowUnchangedWarning(false)} - onSubmitAnyway={() => { + onContinueAnyway={() => { if (taskStatus === "DEFAULT") { - trigger({ - id: frontendId, - update_type: taskType.update_type, - content: replyContent.current, - }); - setTaskStatus("SUBMITTED"); + setTaskStatus("REVIEW"); setShowUnchangedWarning(false); } }} diff --git a/website/src/components/Tasks/TaskTypes.tsx b/website/src/components/Tasks/TaskTypes.tsx index d6f40d0c..6f0bd2e1 100644 --- a/website/src/components/Tasks/TaskTypes.tsx +++ b/website/src/components/Tasks/TaskTypes.tsx @@ -68,7 +68,7 @@ export const TaskTypes: TaskInfo[] = [ type: "rank_prompter_replies", update_type: "message_ranking", unchanged_title: "Order Unchanged", - unchanged_message: "You have not changed the order of the prompts. Are you sure you would like to submit?", + unchanged_message: "You have not changed the order of the prompts. Are you sure you would like to continue?", }, { label: "Rank Assistant Replies", @@ -78,7 +78,7 @@ export const TaskTypes: TaskInfo[] = [ type: "rank_assistant_replies", update_type: "message_ranking", unchanged_title: "Order Unchanged", - unchanged_message: "You have not changed the order of the prompts. Are you sure you would like to submit?", + unchanged_message: "You have not changed the order of the prompts. Are you sure you would like to continue?", }, { label: "Rank Initial Prompts", @@ -88,7 +88,7 @@ export const TaskTypes: TaskInfo[] = [ type: "rank_initial_prompts", update_type: "message_ranking", unchanged_title: "Order Unchanged", - unchanged_message: "You have not changed the order of the prompts. Are you sure you would like to submit?", + unchanged_message: "You have not changed the order of the prompts. Are you sure you would like to continue?", }, // label { diff --git a/website/src/components/Tasks/UnchangedWarning.tsx b/website/src/components/Tasks/UnchangedWarning.tsx index 65d7b50d..8a4039d1 100644 --- a/website/src/components/Tasks/UnchangedWarning.tsx +++ b/website/src/components/Tasks/UnchangedWarning.tsx @@ -14,8 +14,9 @@ interface UnchangedWarningProps { show: boolean; title: string; message: string; + continueButtonText: string; onClose: () => void; - onSubmitAnyway: () => void; + onContinueAnyway: () => void; } export const UnchangedWarning = (props: UnchangedWarningProps) => { @@ -32,7 +33,7 @@ export const UnchangedWarning = (props: UnchangedWarningProps) => { - + From 19e58c04a595567f4e25f12fee587bebfc333e39 Mon Sep 17 00:00:00 2001 From: Adrian Cowan Date: Sat, 14 Jan 2023 02:11:16 +1100 Subject: [PATCH 07/11] website: make e2e tests actually test the tasks work Prior to this change the tests didn't handle any task types as the detection of the task type was failing. --- website/cypress/e2e/tasks/random.cy.ts | 96 ++++++++++--------- .../src/components/Sortable/SortableItem.tsx | 1 + website/src/components/Tasks/LabelTask.tsx | 7 +- 3 files changed, 59 insertions(+), 45 deletions(-) diff --git a/website/cypress/e2e/tasks/random.cy.ts b/website/cypress/e2e/tasks/random.cy.ts index 2ab3bf40..ef970123 100644 --- a/website/cypress/e2e/tasks/random.cy.ts +++ b/website/cypress/e2e/tasks/random.cy.ts @@ -5,56 +5,64 @@ describe("handles random tasks", () => { cy.signInWithEmail("cypress@example.com"); cy.visit("/tasks/random"); - // Check all create tasks. - cy.get('[data-cy="task"]').then((createTaskElement) => { - if (createTaskElement.find('[data-task-type="create-task"]').length) { - cy.get('[data-cy="task-id"]').then((taskIdElement) => { - const taskId = taskIdElement.text(); + // Do some tasks + for (let taskNum = 0; taskNum < 10; taskNum++) { + cy.get('[data-cy="task"]') + .invoke("attr", "data-task-type") + .then((type) => { + cy.log("Task type", type); - const reply = faker.lorem.sentence(); - cy.log("reply", reply); - cy.get('[data-cy="reply"]').type(reply); + cy.get('[data-cy="task-id"]').then((taskIdElement) => { + const taskId = taskIdElement.text(); - cy.get('[data-cy="review"]').click(); + switch (type) { + case "create-task": { + const reply = faker.lorem.sentence(); + cy.log("reply", reply); + cy.get('[data-cy="reply"]').type(reply); - cy.get('[data-cy="submit"]').click(); + cy.get('[data-cy="review"]').click(); - cy.get('[data-cy="task-id]"').should((taskIdElement) => { - expect(taskIdElement.text()).not.to.eq(taskId); + cy.get('[data-cy="submit"]').click(); + break; + } + case "evaluate-task": { + // Rank an item using the keyboard so that the submit button is enabled + cy.get('[aria-roledescription="sortable"]') + .first() + .click() + .type("{enter}") + .wait(100) + .type("{downArrow}") + .wait(100) + .type("{enter}"); + + cy.get('[data-cy="review"]').click(); + + cy.get('[data-cy="submit"]').click(); + + break; + } + case "label-task": { + // Clicking on the slider will set the value to about the middle where it clicks + cy.get('[aria-roledescription="slider"]').first().click(); + + cy.get('[data-cy="review"]').click(); + + cy.get('[data-cy="submit"]').click(); + + break; + } + default: + throw new Error(`Unexpected task type: ${type}`); + } + + cy.get('[data-cy="task-id"]').should((taskIdElement) => { + expect(taskIdElement.text()).not.to.eq(taskId); + }); }); }); - } - - // Check all Evaluate tasks. - if (createTaskElement.find('[data-task-type="evaluate-task"]').length) { - cy.get('[data-cy="task-id"]').then((taskIdElement) => { - const taskId = taskIdElement.text(); - - // Rank an item using the keyboard so that the submit button is enabled - cy.get('button[aria-roledescription="sortable"]') - .first() - .click() - .type("{enter}") - .wait(100) - .type("{downArrow}") - .wait(100) - .type("{enter}"); - - cy.get('[data-cy="review"]').click(); - - cy.get('[data-cy="submit"]').click(); - - cy.get('[data-cy="task-id"]').should((taskIdElement) => { - expect(taskIdElement.text()).not.to.eq(taskId); - }); - }); - } - - if (createTaskElement.find('[data-task-type="label-task"]').length) { - } - - // TODO(#623): Figure out how to fail the test if none of the checks above pass. - }); + } }); }); diff --git a/website/src/components/Sortable/SortableItem.tsx b/website/src/components/Sortable/SortableItem.tsx index 56722694..ca6176c1 100644 --- a/website/src/components/Sortable/SortableItem.tsx +++ b/website/src/components/Sortable/SortableItem.tsx @@ -34,6 +34,7 @@ export const SortableItem = ({ p="4" color={textColor} cursor={isEditable ? (grabbing ? "grabbing" : "grab") : "auto"} + aria-roledescription="sortable" onMouseDown={() => { setGrabbing(true); }} diff --git a/website/src/components/Tasks/LabelTask.tsx b/website/src/components/Tasks/LabelTask.tsx index fe508600..dd66b572 100644 --- a/website/src/components/Tasks/LabelTask.tsx +++ b/website/src/components/Tasks/LabelTask.tsx @@ -118,7 +118,12 @@ function CheckboxSliderItem(props: { {/* TODO: display real text instead of just the id */} {props.labelId} - props.sliderHandler(val / 100)}> + props.sliderHandler(val / 100)} + > From 343853fb57877e3b912dff3232094f840cf2243f Mon Sep 17 00:00:00 2001 From: Andreas Koepf Date: Fri, 13 Jan 2023 15:52:09 +0000 Subject: [PATCH 08/11] add TreeManagerConfiguration as sub-module to global pydantic settings class --- backend/main.py | 7 ++-- backend/oasst_backend/api/v1/tasks.py | 8 ++-- backend/oasst_backend/config.py | 59 ++++++++++++++++++++++++++- backend/oasst_backend/tree_manager.py | 52 ++--------------------- 4 files changed, 66 insertions(+), 60 deletions(-) diff --git a/backend/main.py b/backend/main.py index 76b0a35b..cf0b1f76 100644 --- a/backend/main.py +++ b/backend/main.py @@ -17,7 +17,7 @@ from oasst_backend.config import settings from oasst_backend.database import engine from oasst_backend.models import message_tree_state from oasst_backend.prompt_repository import PromptRepository, TaskRepository, UserRepository -from oasst_backend.tree_manager import TreeManager, TreeManagerConfiguration +from oasst_backend.tree_manager import TreeManager from oasst_shared.exceptions import OasstError, OasstErrorCode from oasst_shared.schemas import protocol as protocol_schema from pydantic import BaseModel @@ -120,7 +120,7 @@ if settings.DEBUG_USE_SEED_DATA: pr = PromptRepository( db=db, api_client=api_client, client_user=dummy_user, user_repository=ur, task_repository=tr ) - tm = TreeManager(db, pr, TreeManagerConfiguration()) + tm = TreeManager(db, pr) with open(settings.DEBUG_USE_SEED_DATA_PATH) as f: dummy_messages_raw = json.load(f) @@ -187,9 +187,8 @@ if settings.DEBUG_USE_SEED_DATA: def ensure_tree_states(): try: logger.info("Startup: TreeManager.ensure_tree_states()") - cfg = TreeManagerConfiguration() # TODO: decide where config is stored, e.g. load form json/yaml file with Session(engine) as db: - tm = TreeManager(db, None, configuration=cfg) + tm = TreeManager(db, None) tm.ensure_tree_states() except Exception: diff --git a/backend/oasst_backend/api/v1/tasks.py b/backend/oasst_backend/api/v1/tasks.py index 4a976f45..17df814f 100644 --- a/backend/oasst_backend/api/v1/tasks.py +++ b/backend/oasst_backend/api/v1/tasks.py @@ -6,7 +6,7 @@ from fastapi.security.api_key import APIKey from loguru import logger from oasst_backend.api import deps from oasst_backend.prompt_repository import PromptRepository, TaskRepository -from oasst_backend.tree_manager import TreeManager, TreeManagerConfiguration +from oasst_backend.tree_manager import TreeManager from oasst_shared.exceptions import OasstError, OasstErrorCode from oasst_shared.schemas import protocol as protocol_schema from sqlmodel import Session @@ -36,8 +36,7 @@ def request_task( try: pr = PromptRepository(db, api_client, client_user=request.user) - tree_manager_config = TreeManagerConfiguration() - tm = TreeManager(db, pr, tree_manager_config) + tm = TreeManager(db, pr) task, message_tree_id, parent_message_id = tm.next_task(request.type) pr.task_repository.store_task(task, message_tree_id, parent_message_id, request.collective) @@ -113,8 +112,7 @@ async def tasks_interaction( try: pr = PromptRepository(db, api_client, client_user=interaction.user) - tree_manager_config = TreeManagerConfiguration() - tm = TreeManager(db, pr, tree_manager_config) + tm = TreeManager(db, pr) return await tm.handle_interaction(interaction) except OasstError: diff --git a/backend/oasst_backend/config.py b/backend/oasst_backend/config.py index 2e2fbf95..0aeea390 100644 --- a/backend/oasst_backend/config.py +++ b/backend/oasst_backend/config.py @@ -1,7 +1,54 @@ from pathlib import Path from typing import Any, Dict, List, Optional, Union -from pydantic import AnyHttpUrl, BaseSettings, FilePath, PostgresDsn, validator +from oasst_shared.schemas import protocol as protocol_schema +from pydantic import AnyHttpUrl, BaseModel, BaseSettings, FilePath, PostgresDsn, validator + + +class TreeManagerConfiguration(BaseModel): + """TreeManager configuration settings""" + + max_active_trees: int = 10 + """Maximum number of concurrently active message trees in the database. + No new initial prompt tasks are handed out to users if this + number is reached.""" + + max_tree_depth: int = 6 + """Maximum depth of message tree.""" + + max_children_count: int = 5 + """Maximum number of reply messages per tree node.""" + + goal_tree_size: int = 15 + """Total number of messages to gather per tree""" + + num_reviews_initial_prompt: int = 3 + """Number of peer review checks to collect in INITIAL_PROMPT_REVIEW state.""" + + num_reviews_reply: int = 3 + """Number of peer review checks to collect per reply (other than initial_prompt)""" + + p_full_labeling_review_prompt: float = 0.1 + """Probability of full text-labeling (instead of mandatory only) for initial prompts""" + + p_full_labeling_review_reply_assistant: float = 0.1 + """Probability of full text-labeling (instead of mandatory only) for assistant replies""" + + p_full_labeling_review_reply_prompter: float = 0.1 + """Probability of full text-labeling (instead of mandatory only) for prompter replies""" + + acceptance_threshold_initial_prompt: float = 0.6 + """Threshold for accepting an initial prompt""" + + acceptance_threshold_reply: float = 0.6 + """Threshold for accepting a reply""" + + num_required_rankings: int = 3 + """Number of rankings in which the message participated.""" + + mandatory_labels_initial_prompt: Optional[list[protocol_schema.TextLabel]] = [protocol_schema.TextLabel.spam] + mandatory_labels_assistant_reply: Optional[list[protocol_schema.TextLabel]] = [protocol_schema.TextLabel.spam] + mandatory_labels_prompter_reply: Optional[list[protocol_schema.TextLabel]] = [protocol_schema.TextLabel.spam] class Settings(BaseSettings): @@ -54,5 +101,13 @@ class Settings(BaseSettings): return v raise ValueError(v) + tree_manager: Optional[TreeManagerConfiguration] = TreeManagerConfiguration() -settings = Settings(_env_file=".env") + class Config: + env_file = ".env" + env_file_encoding = "utf-8" + case_sensitive = False + env_nested_delimiter = "__" + + +settings = Settings() diff --git a/backend/oasst_backend/tree_manager.py b/backend/oasst_backend/tree_manager.py index 8be96aae..6181d62e 100644 --- a/backend/oasst_backend/tree_manager.py +++ b/backend/oasst_backend/tree_manager.py @@ -8,7 +8,7 @@ import numpy as np import pydantic from loguru import logger from oasst_backend.api.v1.utils import prepare_conversation, prepare_conversation_message_list -from oasst_backend.config import settings +from oasst_backend.config import TreeManagerConfiguration, settings from oasst_backend.models import Message, MessageReaction, MessageTreeState, TextLabels, message_tree_state from oasst_backend.prompt_repository import PromptRepository from oasst_backend.utils.hugging_face import HfEmbeddingModel, HfUrl, HuggingFaceAPI @@ -18,52 +18,6 @@ from sqlalchemy.sql import text from sqlmodel import Session, func -class TreeManagerConfiguration(pydantic.BaseModel): - """TreeManager configuration settings""" - - max_active_trees: int = 10 - """Maximum number of concurrently active message trees in the database. - No new initial prompt tasks are handed out to users if this - number is reached.""" - - max_tree_depth: int = 6 - """Maximum depth of message tree.""" - - max_children_count: int = 5 - """Maximum number of reply messages per tree node.""" - - goal_tree_size: int = 15 - """Total number of messages to gather per tree""" - - num_reviews_initial_prompt: int = 3 - """Number of peer review checks to collect in INITIAL_PROMPT_REVIEW state.""" - - num_reviews_reply: int = 3 - """Number of peer review checks to collect per reply (other than initial_prompt)""" - - p_full_labeling_review_prompt: float = 0.1 - """Probability of full text-labeling (instead of mandatory only) for initial prompts""" - - p_full_labeling_review_reply_assistant: float = 0.1 - """Probability of full text-labeling (instead of mandatory only) for assistant replies""" - - p_full_labeling_review_reply_prompter: float = 0.1 - """Probability of full text-labeling (instead of mandatory only) for prompter replies""" - - acceptance_threshold_initial_prompt: float = 0.6 - """Threshold for accepting an initial prompt""" - - acceptance_threshold_reply: float = 0.6 - """Threshold for accepting a reply""" - - num_required_rankings: int = 3 - """Number of rankings in which the message participated.""" - - mandatory_labels_initial_prompt: Optional[list[protocol_schema.TextLabel]] = [protocol_schema.TextLabel.spam] - mandatory_labels_assistant_reply: Optional[list[protocol_schema.TextLabel]] = [protocol_schema.TextLabel.spam] - mandatory_labels_prompter_reply: Optional[list[protocol_schema.TextLabel]] = [protocol_schema.TextLabel.spam] - - class TaskType(Enum): NONE = -1 RANKING = 0 @@ -114,9 +68,9 @@ class IncompleteRankingsRow(pydantic.BaseModel): class TreeManager: _all_text_labels = list(map(lambda x: x.value, protocol_schema.TextLabel)) - def __init__(self, db: Session, prompt_repository: PromptRepository, configuration: TreeManagerConfiguration): + def __init__(self, db: Session, prompt_repository: PromptRepository): self.db = db - self.cfg = configuration + self.cfg = settings.tree_manager self.pr = prompt_repository def _task_selection( From d0ef7c4bbccc1bae882006e2143480bbd5f38d6c Mon Sep 17 00:00:00 2001 From: Andreas Koepf Date: Fri, 13 Jan 2023 16:04:06 +0000 Subject: [PATCH 09/11] add missing doc-strings for tree-manager mandatory labels settings --- backend/oasst_backend/config.py | 5 +++++ backend/oasst_backend/tree_manager.py | 6 ++++-- 2 files changed, 9 insertions(+), 2 deletions(-) diff --git a/backend/oasst_backend/config.py b/backend/oasst_backend/config.py index 0aeea390..c18bd4c2 100644 --- a/backend/oasst_backend/config.py +++ b/backend/oasst_backend/config.py @@ -47,8 +47,13 @@ class TreeManagerConfiguration(BaseModel): """Number of rankings in which the message participated.""" mandatory_labels_initial_prompt: Optional[list[protocol_schema.TextLabel]] = [protocol_schema.TextLabel.spam] + """Mandatory labels in text-labeling tasks for initial prompts.""" + mandatory_labels_assistant_reply: Optional[list[protocol_schema.TextLabel]] = [protocol_schema.TextLabel.spam] + """Mandatory labels in text-labeling tasks for assistant reylies.""" + mandatory_labels_prompter_reply: Optional[list[protocol_schema.TextLabel]] = [protocol_schema.TextLabel.spam] + """Mandatory labels in text-labeling tasks for prompter replies.""" class Settings(BaseSettings): diff --git a/backend/oasst_backend/tree_manager.py b/backend/oasst_backend/tree_manager.py index 6181d62e..d7abd9f8 100644 --- a/backend/oasst_backend/tree_manager.py +++ b/backend/oasst_backend/tree_manager.py @@ -68,9 +68,11 @@ class IncompleteRankingsRow(pydantic.BaseModel): class TreeManager: _all_text_labels = list(map(lambda x: x.value, protocol_schema.TextLabel)) - def __init__(self, db: Session, prompt_repository: PromptRepository): + def __init__( + self, db: Session, prompt_repository: PromptRepository, cfg: Optional[TreeManagerConfiguration] = None + ): self.db = db - self.cfg = settings.tree_manager + self.cfg = cfg or settings.tree_manager self.pr = prompt_repository def _task_selection( From a9664172acab729f7e6095340373c3e8f731f958 Mon Sep 17 00:00:00 2001 From: Vechtomov Date: Fri, 13 Jan 2023 23:18:01 +0300 Subject: [PATCH 10/11] add protected routes --- website/src/middleware.ts | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/website/src/middleware.ts b/website/src/middleware.ts index d1cd6801..21eeaaa7 100644 --- a/website/src/middleware.ts +++ b/website/src/middleware.ts @@ -4,5 +4,15 @@ export { default } from "next-auth/middleware"; * Guards these pages and redirects them to the sign in page. */ export const config = { - matcher: ["/create/:path*", "/evaluate/:path*", "/label/:path*", "/account/:path*", "/dashboard", "/admin/:path*"], + matcher: [ + "/create/:path*", + "/evaluate/:path*", + "/label/:path*", + "/account/:path*", + "/dashboard", + "/admin/:path*", + "/tasks/:path*", + "/leaderboard", + "/messages/:path*", + ], }; From d9bf43ea1570cfc6c01410e6b9f5b10d864a225d Mon Sep 17 00:00:00 2001 From: klotske Date: Fri, 13 Jan 2023 23:44:15 +0300 Subject: [PATCH 11/11] Introduce sortable bounds --- website/src/components/Sortable/Sortable.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/website/src/components/Sortable/Sortable.tsx b/website/src/components/Sortable/Sortable.tsx index 48c2916b..f77f9dd2 100644 --- a/website/src/components/Sortable/Sortable.tsx +++ b/website/src/components/Sortable/Sortable.tsx @@ -9,7 +9,7 @@ import { useSensors, } from "@dnd-kit/core"; import type { DragEndEvent } from "@dnd-kit/core/dist/types/events"; -import { restrictToVerticalAxis } from "@dnd-kit/modifiers"; +import { restrictToParentElement, restrictToVerticalAxis } from "@dnd-kit/modifiers"; import { arrayMove, SortableContext, @@ -60,7 +60,7 @@ export const Sortable = (props: SortableProps) => { sensors={sensors} collisionDetection={closestCenter} onDragEnd={handleDragEnd} - modifiers={[restrictToVerticalAxis]} + modifiers={[restrictToParentElement, restrictToVerticalAxis]} >