-
Notifications
You must be signed in to change notification settings - Fork 481
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
Handle pending and error states for rubric ai evaluation #54860
Handle pending and error states for rubric ai evaluation #54860
Conversation
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.
hi Mark, just a couple of comments, but I'd like to look again after you are using the shared constants.
since you are already doing the work of locating the RubricAiEvaluation and sending its status to the client, I would suggest also handling the pending state in this PR. this has 2 parts:
- (more important) the teacher click Run AI, and we use the evaluation status (rather than the other handwavy logic about attempted and last_attempt_evaluated) to determine whether the request is pending
- the teacher gets bored and refreshes the page while the ai eval is running. we should show them the pending state correctly on page load, rather than allowing them to click Run AI again
if you want this to be in a separate PR that is ok too -- the error state is most important because teachers won't likely get bored and refresh too often.
setStatus(STATUS.ERROR); | ||
} else if (data.status === 1001) { | ||
setStatus(STATUS.PII_ERROR); | ||
} else if (data.status === 1002) { |
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.
please use the js version of these shared constants, here and below. for example:
import {RubricUnderstandingLevels} from '@cdo/apps/util/sharedConstants'; return aiUnderstandingLevel >= RubricUnderstandingLevels.CONVINCING
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.
fixed
@@ -91,9 +105,17 @@ export default function RubricSettings({ | |||
setStatus(STATUS.ERROR); | |||
} else { | |||
response.json().then(data => { | |||
if (data.lastAttemptEvaluated) { | |||
console.log('2nd'); | |||
console.log(data); |
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.
remove debug 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.
fixed
expect(wrapper.find('Button').props().disabled).to.be.true; | ||
expect(wrapper.text()).include(i18n.aiEvaluationStatus_pending()); | ||
expect(wrapper.text()).include(i18n.aiEvaluationStatus_in_progress()); |
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.
style nit: I realize you're just following an existing pattern here, but in the future, please use camelCase for js variable names where possible, e.g. aiEvaluationStatusInProgress
.
Most of our style rules are automatically enforced by prettier. the rest should be documented here: https://github.com/code-dot-org/code-dot-org/blob/staging/STYLEGUIDE.md
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.
No problem. Yeah, I was just matching the pattern from the pervious strings.
* Tutorial explorer: preview mode on hourofcode.com * Updated test to account for flakiness. * Added comment * Re-enabled test * hoc2023: AI string updates * Use use_preview variable * Update schema cache dump after schema changes. * Require script_id for SingleSectionExperiment and TeacherBasedExperiment (#54843) * require script_id on SingleSectionExperiment and TeacherBasedExperiment * fix and improve unit tests * fix unit tests * revert stray schema changes * Update string and font size * Split lines for explanation text. * update injection options (#54818) * Handle pending and error states for rubric ai evaluation (#54860) * updated rubric for more useful error messages * updated rubric settings tests for new errors * undo debug changes to rubrics_controller * bug fixes * added messages for pending and running states * Dance AI: Add start over button (#54899) * Dance AI: Add start over button * don't show on explanation page, add tooltips * Updates to instructions in HoC progression (#54881) Updates to instructions in HoC progression * Allow setting workshop organizer permission (#54896) * Allow setting workshop organizer permission * Update Assignable Permissions * added categories and standards csv files (#54933) * levelbuilder content changes (-robo-commit) * staging content changes (-robo-commit) * * removed empty files on dashboard/public/fonts/gotham (#54906) * P20-522: Fix Express22 level (#54789) * P20-522: Translate `gamelab_ifVarEquals` block type NUM vsriables * P20-522: Translate `procedures_defnoreturn` block id attr * Revert "P20-522: Translate `procedures_defnoreturn` block id attr" This reverts commit e0f4280. * [DESign2-51] Add Chips component (#54064) * Created Chips component * created stories for Chips * created tests for Chips * created documentation for Chips * removed MultiSelectGroup * used Chips instead of MultiSelectGroup in the code * Teachers find course button goes to catalog (#54918) * Teachers find course button goes to catalog * update test * staging content changes (-) * HoC 2023 - Fix missing string on Educators How-to guide (#54932) * Add skinny banner to code.org/student/middle-high for AFE scholarships (#54907) * refactor middle-high.haml and remove stylesheet * add afe_scholarship_skinny_banner.haml view * update grade labels and add empty alt text to images * Dance AI: handle RTL in header, footer, and effect/code toggle (#54922) * * add focus state for toggle code/effect buttons * * dance ai - explanation area RTL support fixes * * dance party - song selector RTL support fix * * updated explanation button rtl languages positioning * added comment (review fix) * Center toggle in LTR/RTL, force LTR header even in RTL languages * RTL header * Handle RTL in footer buttons * Fix text/icon spacing in RTL * Undo spacing change * Use div instead of span --------- Co-authored-by: denyslevada <levada.denys@gmail.com> * standardize counting of events so all are right (#54927) * Update Neural Networks lesson text on code.org/ai/how-ai-works and hourofcode.com/ai (#54915) * Update code.org/hourofcode see all Hour of code tutorials section (#54931) * add see all activities section * update images and add responsive styles --------- Co-authored-by: breville <brendan@code.org> Co-authored-by: Jessica Kulwik <jessica@code.org> Co-authored-by: Dave Bailey <davidsbailey@users.noreply.github.com> Co-authored-by: Molly Moen <molly@code.org> Co-authored-by: Mark Barnes <mark.barnes@code.org> Co-authored-by: Sanchit Malhotra <85528507+sanchitmalhotra126@users.noreply.github.com> Co-authored-by: fisher-alice <107423305+fisher-alice@users.noreply.github.com> Co-authored-by: Brendan Reville <breville@users.noreply.github.com> Co-authored-by: Dani LaMarca <dani@code.org> Co-authored-by: levadadenys <levada.denys@gmail.com> Co-authored-by: Artem Vavilov <11708250+artem-vavilov@users.noreply.github.com> Co-authored-by: Kelby Hawn <9256643+kelbyhawn@users.noreply.github.com> Co-authored-by: bencodeorg <ben@code.org> Co-authored-by: Turner Riley <56283563+TurnerRiley@users.noreply.github.com>
Updated RubricSettings and rubrics_controller components to handle error messages for PII, profanity, and general failures.
General Error Message
PII Error Message
Profanity Filter Error Message