Add feedback component #793

Etesam913 · 2022-06-14T15:25:28Z

Summary

Adds feedback component to mephisto-worker-addons.

Properties for this component are defined in the mephisto-worker-addons readme.

There is a questions prop that allows for questions to be associated with a feedback textarea when reviewing.

Uses the handleSubmitMetadata() method from mephisto-task in the default handleSubmit() of the Feedback component to make submitting feedback very easy. Very similar to how it is done in the Tips component

Adds feedback component to static_react_task_with_tips webapp

Created a reducer to add error checking for both the feedback & tips components.

Added some small changes like new properties for the Tips component.

Test Plan

New video:

#793 (comment)

Old Video:

feedback-demo-with-questions.mov

In the video it should be clear that I write feedback for the two questions. The feedback for the first question is clearly not toxic, while the feedback for the second question clearly is.

Then I ran the review_feedback_for_task.py script where I was first able to filter by the question asked. Then I was able to filter by toxicity & then filter by unreviewed/reviewed feedback.

I ran the script three times.

The first time I reviewed the feedback for the first question. The toxicity of the feedback was < 0.5, so filtering by toxicity would do nothing.

The second time I reviewed the feedback for the other question. I chose to filter by toxicity and the feedback message did not show up as it's toxicity > 0.5.

The third time I reviewed the same question as the second time, but this time I chose not to filter by toxicity. As expected, it showed the message.

TODO:

Make detoxify get installed when pip install -e . is ran in root
Planning on publishing the changes to mephisto-task and mephisto-worker-addons after this pull request gets merged

📝 Updated mephisto-worker-experience with usage of Feedback

📝 Added documentation for width prop

🏷️ Added return types to review_tips_for_task.py ✨ Added feedback tag to tips example ✨ UUpdated ClientIOHandler to handle packet details for feedback

✏️ Updated some of the print statements to be more clear/have more spacing

✨ Added a deffault length limit to feedback textarea

🎨 Changed width property to represent the width of the textarea.

📝 Uppdated readme to reflect these new props.

♻️ Refactored some of the user script code by putting acceptable responses in a set

✅ Modified spacing in Tips/index.js to pass test

🚛 Moved submit button into its own file 🎨 Added a red border for textareas when there is an error 🔥 Removed observer code for error popup as textarea resize direction was changed to exclusively vertical 📝 Updated docstrings for functions

🔥 Removed createTip and createFeedback from exports, might revisit later

Etesam913

Added some explanatory comments on what I have done

Etesam913 · 2022-07-14T20:03:19Z

examples/static_react_task_with_tips/webapp/src/components/core_components.jsx

+            <Feedback
+              maxTextLength={30}
+              questions={[
+                "What is your favorite part of this task?",
+                "Were you satisfied with this task?",
+              ]}
+            />


The new Feedback component, you can optionally pass in questions or have no questions and just have general feedback.

Etesam913 · 2022-07-14T20:05:15Z

mephisto/data_model/agent.py

+                feedback_to_add = {
+                    "id": str(uuid4()),
+                    "question": question_obj["question"],
+                    "text": new_feedback_text,
+                    "reviewed": False,
+                    "toxicity": str(new_feedback_toxicity),
+                }


New feedback object that gets added

Etesam913 · 2022-07-14T20:05:58Z

mephisto/scripts/local_db/review_feedback_for_task.py

+"""
+Script that allows for feedback to be reviewed.
+Feedback are collected by retrieving all units and then getting the agent of each unit.
+The agent state has the feedback data.
+
+When running the script you first have the option of
+filtering out potential toxic feedback.
+After that you have the ability to view reviewed or unreviewed feedback.
+
+If you select reviewed feedback, then the script will print out all of the reviewed feedback.
+If you select unreviewed feedback, then the script will print out each
+unreviewed feedback and give you the option to mark the feedback as reviewed.
+"""


Most of this script gets reworked with rich in #816

Etesam913 · 2022-07-14T20:07:47Z

packages/mephisto-worker-addons/README.md

+### `maxHeaderLength`
+The max character length of a tip header before an error message is shown. The default value for this prop is 72.
+### `maxTextLength`
+The max character length of tip text before an error message is shown. The default value for this prop is 500.


Added some new props for tips in this pr.

If length exceeds the max lengths an error message shows up that prevents the submission of the tips

Etesam913 · 2022-07-14T20:08:09Z

packages/mephisto-worker-addons/README.md

+## Documentation
+### `headless`
+The `headless` prop accepts a boolean where a a true value removes most of the styling and a false value keeps the original styling. The default value of this prop is false.
+### `questions`
+The `questions` prop accepts a list of strings where each string is displayed with its own textarea input. When reviewing the feedback in the task, there is an option to filter by question. By default there are no questions defined and feedback is collected under the 'General Feedback' label.
+### `handleSubmit`
+The `handleSubmit` prop accepts a function that runs when the "Submit Feedback" button is pressed instead of the default behavior of submitting feedback. The text property can be passed down through to the function.
+### `textAreaWidth`
+The `textAreaWidth` prop accepts a string that sets the width of the text-area. ex: textAreaWidth="30rem" sets the text-area width. The default value for this prop is 100%.
+### `maxTextLength`
+The max character length of feedback text before an error message is shown. The default value for this prop is 700.


Feedback properties

Etesam913 · 2022-07-14T20:09:34Z

packages/mephisto-worker-addons/src/Feedback/Question.js

+      <FeedbackTextArea
+        id={`question-${index}`}
+        ref={ref}
+        index={index}
+        width={textAreaWidth}
+        questionsFeedbackText={questionsFeedbackText}
+        feedbackText={questionsFeedbackText[index]}
+        setFeedbackText={(textValue) => {
+          const tempQuestionsFeedback = [...questionsFeedbackText];
+          tempQuestionsFeedback[index] = textValue;
+          setQuestionsFeedbackText(tempQuestionsFeedback);
+        }}
+        stylePrefix={stylePrefix}
+        state={state}
+        dispatch={dispatch}
+        maxFeedbackLength={maxFeedbackLength}
+        containsQuestions
+        placeholder={placeholder}
+      />


These are a lot of props for some of these components, but using context or a state management lib are probably overkill for this package.

Etesam913 · 2022-07-14T20:11:11Z

packages/mephisto-worker-addons/src/Feedback/index.js

+  useEffect(() => {
+    if (questions) setQuestionsFeedbackText(questions.map(() => ""));
+  }, [questions]);


initialize with correct length

Etesam913 · 2022-07-14T20:11:52Z

packages/mephisto-worker-addons/src/Feedback/index.js

+  const [state, dispatch] = useReducer(feedbackReducer, {
+    status: 0,
+    text: "",
+    errorIndexes: null,
+  });


Handles different states that the feedback component can be in (error, loading, normal, etc...)

Etesam913 · 2022-07-14T20:16:05Z

packages/mephisto-worker-addons/src/index.jsx

-import { createFeedback, createTip } from "./Functions";
-export { Tips, createFeedback, createTip };
+import Feedback from "./Feedback/index";
+export { Tips, Feedback };


The createFeedback & createTip methods are way more involved/complex than they previously were, so it makes not sense for them to be exported for the user to run.

Etesam913 · 2022-07-14T20:19:29Z

Tests are failing because detoxify is not installed globally

🎨 Improved layout of feedback and tips in task

Etesam913 · 2022-07-14T21:11:23Z

Updated video:

updated-feedback-component.mov

JackUrb · 2022-07-15T14:44:08Z

Can you merge #779 into this and then set it as a base branch for this PR? Should let you then add detoxify as a dependency in pyproject.toml.

Etesam913 · 2022-07-15T14:48:11Z

Can you merge #779 into this and then set it as a base branch for this PR? Should let you then add detoxify as a dependency in pyproject.toml.

Wouldn't setting #779 as the base branch for #793 just make this code go into #779 once #793 is merged? Do we want that?

I also have to add it to requirements.txt as well right?

…phisto into add-feedback-component

JackUrb · 2022-07-15T14:57:34Z

requirements.txt no longer exists after that PR.

JackUrb · 2022-07-15T14:58:20Z

And ideally we'll be merging #779 in first! Setting the base branch in this case just makes review easier by isolating the changeset.

Etesam913 · 2022-07-15T14:59:48Z

Okay, so I guess I just have to remember to switch the base branch back to main before merging this pr in.

codecov-commenter · 2022-07-15T15:08:29Z

Codecov Report

Merging #793 (4c284b9) into main (284b872) will decrease coverage by 0.11%.
The diff coverage is 23.80%.

@@            Coverage Diff             @@
##             main     #793      +/-   ##
==========================================
- Coverage   64.62%   64.50%   -0.12%     
==========================================
  Files         107      107              
  Lines        9264     9281      +17     
==========================================
  Hits         5987     5987              
- Misses       3277     3294      +17

Impacted Files	Coverage Δ
mephisto/tools/data_browser.py	`40.98% <ø> (ø)`
mephisto/data_model/agent.py	`75.07% <16.66%> (-2.27%)`	⬇️
...ephisto/abstractions/_subcomponents/agent_state.py	`79.83% <66.66%> (-0.35%)`	⬇️
...tractions/architects/channels/websocket_channel.py	`67.96% <0.00%> (-8.60%)`	⬇️
mephisto/data_model/unit.py	`78.14% <0.00%> (+0.54%)`	⬆️
mephisto/data_model/assignment.py	`61.71% <0.00%> (+3.90%)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 284b872...4c284b9. Read the comment docs.

JackUrb

Only small comments on the backend, the overall implementation looks great!

As an aside, I imagine it would be really cool to provide a navbar-style meta-component that can hold these types of add-ons behind button toggles in an attached overlay. (similar to how tips already works!)

I'll hold off on stamping to merge until tests are passing and the underlying PR is in.

JackUrb · 2022-07-15T14:52:15Z

mephisto/data_model/agent.py

@@ -30,6 +30,7 @@
 )

 from typing import Optional, Mapping, Dict, Any, cast, TYPE_CHECKING
+from detoxify import Detoxify


I feel like Detoxify is a pretty heavy import for the Agent class, as well as one that we may allow people to skip. For this we can do an import flag:

try: from detoxify import Detoxify DETOXIFY_INSTALLED = True except ImportError: DETOXIFY_INSTALLED = False

Then later in the elif "feedback" in data: block you can use Detoxify if DETOXIFY_INSTALLED else use some kind of null or not evaluated value for new_feedback_toxicity.

That certainly is a way of doing it. Something that I saw in the codebase was putting imports inside an if statement

elif "feedback" in data: from detoxify import Detoxify ...

Would this be a fix to this problem as well?

I was considering this approach as well, though it makes the choice less deliberate. I tend to only use it when there are potential dependency loops. It also means if someone wanted to drop detoxify they'd have to patch here, while someone wanting a lightweight system could benefit from just not installing detoxify.

Of course, there may be the case of someone who already has detoxify installed but who doesn't want to use it here, but I'm not terribly concerned there.

JackUrb · 2022-07-15T15:00:11Z

mephisto/scripts/local_db/review_feedback_for_task.py

+    print("\nTask Names:")
+    for task_name in task_names:
+        print(task_name)
+
+    # Getting the task name
+    task_name = input(
+        "\nEnter the name of the task that you want to review the feedback of: \n"
+    )
+    print("")
+    while task_name not in task_names:
+        print("That task name is not valid\n")
+        task_name = input(
+            "Enter the name of the task that you want to review the feedback of: \n"
+        )
+        print("")


As an aside, this style of task name selection is a pretty common utility, and would likely be a good candidate to abstract into the mephisto.tools.scripts collection. Can do after #816

mephisto/scripts/local_db/review_tips_for_task.py

JackUrb · 2022-07-15T15:08:48Z

pyproject.toml

@@ -51,6 +51,7 @@ hydra-core = "^1.1.0"
 tqdm = "^4.50.2"
 websockets = "^10.1"
 pyyaml = "^5.4.0"
+detoxify = "^0.5.0"


Just for organization, can you add this under a # [tool.poetry.group.worker_addons.dependencies] heading? I'll be toggling these in poetry 1.2.

ℹ️ Feedback submitted without the detoxify dependency has None in its toxicity field. When reviewing this feedback submission filtering by toxicity will have no effect on if it shows or not. 💡 Updated the main comment in review_tips_for_task.py

JackUrb

I think this is ready to go. Thanks for the patience here in getting this right @Etesam913!

Etesam913 · 2022-07-18T14:32:42Z

@pringshia I should probably make the non-headless version of this component be in a shadow-dom right?

…nto add-feedback-component

pringshia · 2022-07-18T16:02:55Z

@pringshia I should probably make the non-headless version of this component be in a shadow-dom right?

Yep, thanks!

Etesam913 added 11 commits June 12, 2022 22:05

🚧 Setup baseline Feedback component

0cab918

🎨 Applied final styling to Feedback component

1ffe2b0

📝 Updated mephisto-worker-experience with usage of Feedback

✨ Added width prop to Feedback component

8b9858c

📝 Added documentation for width prop

✨ Added feedback component to example task

d230cf3

🚧 First iteration of review_feedback_for_task.py

f8fb47b

🏷️ Added return types to review_tips_for_task.py ✨ Added feedback tag to tips example ✨ UUpdated ClientIOHandler to handle packet details for feedback

🐛 Fixed bug where feedback was not set as reviewed

2a06017

✏️ Updated some of the print statements to be more clear/have more spacing

🔨 Updated hydra config file for tips-example

6b73014

🥅 Added error handling to feedback

d1d2a1d

✨ Added a deffault length limit to feedback textarea

🥅 Added size error handling for tips

902f72c

🐛 Fixed layout issues with feedback error message

e99fb79

🎨 Changed width property to represent the width of the textarea.

✨ Added ability to set maxTextLengths

68ca650

📝 Uppdated readme to reflect these new props.

facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Jun 14, 2022

Etesam913 added 13 commits June 14, 2022 14:02

✨ Added toxicity property to feedback

9082b6e

✨ Added ability to filter feedback by toxicity

7f310be

♻️ Refactored some of the user script code by putting acceptable responses in a set

🔀 Merged add-feedback-component with main

21e71f3

✅ Modified spacing in Tips/index.js to pass test

🐛 Fixed bug with some strings not being recognized

9a21622

🐛 Fixed bug with feedback not being a str

30b3b62

🔀 Merged with add-tips-example

5765e2a

🔀 Merged with add-cypress-to-static-task

0f0179c

🔀 Merged with add-tips-example

14439ea

✏️ Changed classnames of feedback component

6468487

🚧 Began work on adding questions prop

5aa4b8d

🎨 Made touchups to questions in feedback

f309aca

🏗 Continued work on adding questions to feedback

2731651

pringshia mentioned this pull request Jun 28, 2022

Review outstanding PRs for Cypress integration and the Feedback/Tips component #807

Closed

Etesam913 added 4 commits June 28, 2022 15:51

🎨 Reworked createFeedback() for it to handle question answers

0311bc9

✨ Added ability to filter by question when reviewing

5cb173a

↩️ Reverted weird changes to server.js test file

e17a7aa

📝 Added documentation on questions prop

57c5b47

🔥 Removed createTip and createFeedback from exports, might revisit later

Etesam913 added 3 commits July 10, 2022 14:44

🐛 Made FeedbackTextArea disabled when loading

3932d3c

🔀 Merged with add-tips-example

ba79b8f

🔀 Merged with main

f1a1aa7

Etesam913 commented Jul 14, 2022

View reviewed changes

🐛 Fixed bug with feedback submission

2d22537

🎨 Improved layout of feedback and tips in task

Etesam913 requested review from JackUrb and pringshia July 15, 2022 14:37

Merge branch 'version-bump' of https://github.com/facebookresearch/Me…

5c9f0d8

…phisto into add-feedback-component

JackUrb changed the base branch from main to version-bump July 15, 2022 14:56

➕ Added detoxify to pyproject.toml

ed39799

JackUrb reviewed Jul 15, 2022

View reviewed changes

➕ Adds worker_addons.dependencies section to pyproject

8732bd5

Base automatically changed from version-bump to main July 15, 2022 17:57

Etesam913 added 2 commits July 17, 2022 12:40

🔀 Merged with main

a09ce88

JackUrb approved these changes Jul 18, 2022

View reviewed changes

Etesam913 added 2 commits July 18, 2022 11:09

Merge branch 'main' of https://github.com/facebookresearch/Mephisto i…

4ccb26c

…nto add-feedback-component

🎨 Non-headless version of feedback in shadow-dom

8feb698

Etesam913 merged commit be8b872 into main Jul 18, 2022

Etesam913 deleted the add-feedback-component branch July 19, 2022 19:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add feedback component #793

Add feedback component #793

Etesam913 commented Jun 14, 2022 •

edited

Loading

Etesam913 left a comment

Etesam913 Jul 14, 2022 •

edited

Loading

Etesam913 Jul 14, 2022

Etesam913 Jul 14, 2022

Etesam913 Jul 14, 2022 •

edited

Loading

Etesam913 Jul 14, 2022

Etesam913 Jul 14, 2022 •

edited

Loading

Etesam913 Jul 14, 2022

Etesam913 Jul 14, 2022

Etesam913 Jul 14, 2022

Etesam913 commented Jul 14, 2022

Etesam913 commented Jul 14, 2022

JackUrb commented Jul 15, 2022

Etesam913 commented Jul 15, 2022 •

edited

Loading

JackUrb commented Jul 15, 2022

JackUrb commented Jul 15, 2022 •

edited

Loading

Etesam913 commented Jul 15, 2022

codecov-commenter commented Jul 15, 2022 •

edited

Loading

JackUrb left a comment

JackUrb Jul 15, 2022

Etesam913 Jul 15, 2022 •

edited

Loading

JackUrb Jul 15, 2022 •

edited

Loading

JackUrb Jul 15, 2022

JackUrb Jul 15, 2022

JackUrb left a comment

Etesam913 commented Jul 18, 2022

pringshia commented Jul 18, 2022

Add feedback component #793

Add feedback component #793

Conversation

Etesam913 commented Jun 14, 2022 • edited Loading

Summary

Test Plan

New video:

Old Video:

TODO:

Etesam913 left a comment

Choose a reason for hiding this comment

Etesam913 Jul 14, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Etesam913 Jul 14, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Etesam913 Jul 14, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Etesam913 commented Jul 14, 2022

Etesam913 commented Jul 14, 2022

JackUrb commented Jul 15, 2022

Etesam913 commented Jul 15, 2022 • edited Loading

JackUrb commented Jul 15, 2022

JackUrb commented Jul 15, 2022 • edited Loading

Etesam913 commented Jul 15, 2022

codecov-commenter commented Jul 15, 2022 • edited Loading

Codecov Report

JackUrb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Etesam913 Jul 15, 2022 • edited Loading

Choose a reason for hiding this comment

JackUrb Jul 15, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JackUrb left a comment

Choose a reason for hiding this comment

Etesam913 commented Jul 18, 2022

pringshia commented Jul 18, 2022

Etesam913 commented Jun 14, 2022 •

edited

Loading

Etesam913 Jul 14, 2022 •

edited

Loading

Etesam913 Jul 14, 2022 •

edited

Loading

Etesam913 Jul 14, 2022 •

edited

Loading

Etesam913 commented Jul 15, 2022 •

edited

Loading

JackUrb commented Jul 15, 2022 •

edited

Loading

codecov-commenter commented Jul 15, 2022 •

edited

Loading

Etesam913 Jul 15, 2022 •

edited

Loading

JackUrb Jul 15, 2022 •

edited

Loading