trial runner page pt 2 #12517

emilia-friedberg · 2024-11-01T15:58:38Z

WHAT

Part two of the trial runner -- I've decided to split this up yet again because this section required a refactor of some of our existing components, which are now shared. The big change here is that we're no longer reloading the page, but setting the results of new get methods when guidelines and dataset_relevant_texts are updated into the parent components, and invalidating the caches in the background. window.reload isn't a big deal on largely static pages, but since the trial runner is essentially one big form, I didn't want to risk users losing information they'd already input if they are making changes in one of these areas.

WHY

This is a requisite part of the work for the trial runner.

HOW

Add various fields to the trial runner, and repurpose existing components for this page as well. This involved some backend work to add and test new index functions for the relevant controllers.

Screenshots

Notion Card Links

https://www.notion.so/quill/Trial-Runner-Page-12ed42e6f9418002b2f0e2e7917a8117?pvs=4

What have you done to QA this feature?

Tested the new flows, including that updating guidelines and dataset relevant texts on one page correctly updates them on the other. Jamie will QA when this card is complete.

PR Checklist	Your Answer
Have you added and/or updated tests?	YES
Have you deployed to Staging?	Tested locally, will deploy when the rest of the page is in place.
Self-Review: Have you done an initial self-review of the code below on Github?	YES

emilia-friedberg · 2024-11-01T17:49:36Z

.../client/app/bundles/Staff/components/evidence/llmPromptDatasets/shared/guidelinesSection.tsx

@@ -0,0 +1,232 @@
+import * as React from "react";


This file is mostly not new -- I'm not sure why it's diffing that way. It was moved from the individualDataset folder and I refactored the table to allow for rows to be selectable where it's used in the TrialRunner.

anathomical

I like the move away from reload. That's always felt like a sort of clunky solution, so yay for the new one.

Beyond that, I've got a couple of minor style flags, but overall this makes sense to me. I also appreciate how you've broken these up into smaller chunks which makes it a lot easier to review.

anathomical · 2024-11-01T19:26:57Z

...gines/evidence/app/controllers/evidence/research/gen_ai/dataset_relevant_texts_controller.rb

@@ -4,6 +4,10 @@ module Evidence
  module Research
    module GenAI
      class DatasetRelevantTextsController < ApplicationController
+        def index
+          render json: { dataset_relevant_texts: dataset.dataset_relevant_texts.order(:id) }


I like the explicit ordering by id here. I think it's smart not to leave that to chance.

anathomical · 2024-11-01T19:27:44Z

.../QuillLMS/engines/evidence/app/controllers/evidence/research/gen_ai/guidelines_controller.rb

@@ -4,6 +4,10 @@ module Evidence
  module Research
    module GenAI
      class GuidelinesController < ApplicationController
+        def index
+          render json: { guidelines: dataset.guidelines }


It might be worth including that order(:id) clause here, too, though given the dataset context, that may not matter as much.

anathomical · 2024-11-01T19:29:49Z

...evidence/spec/controllers/evidence/research/gen_ai/dataset_relevant_texts_controller_spec.rb

+        def parsed_response
+          JSON.parse(response.body)
+        end


I don't think this is a huge deal, since it's just style, so take or leave, but you can also use the let syntax for this: let(:parsed_response) { JSON.parse(response.body) }. I think that's slightly more in-line with our existing patterns, but like I said it's fine as-is.

anathomical · 2024-11-01T19:35:59Z

...llLMS/engines/evidence/spec/controllers/evidence/research/gen_ai/datasets_controller_spec.rb

+        def parsed_response
+          JSON.parse(response.body)
+        end


Again could use let here.

anathomical · 2024-11-01T19:36:53Z

...LMS/engines/evidence/spec/controllers/evidence/research/gen_ai/guidelines_controller_spec.rb

+        def parsed_response
+          JSON.parse(response.body)
+        end


Again, could use let here if you like.

eadams17

LGTM!

brendanshean

Nice work on this. Thank you for the explanation on the move away from window.reload

brendanshean · 2024-11-04T17:36:16Z

.../bundles/Staff/components/evidence/llmPromptDatasets/individualDataset/individualDataset.tsx

@@ -53,13 +52,12 @@ const IndividualDataset: React.FC<RouteComponentProps<ActivityRouteProps>> = ({

  React.useEffect(() => {
    if (loading && datasetData?.data) {
-      const { dataset, data_subsets, stem_vault, trials, dataset_relevant_texts, optimal_guidelines, suboptimal_guidelines } = datasetData.data;


I like the refactor to just guidelines

brendanshean · 2024-11-04T17:37:14Z

...dles/Staff/components/evidence/llmPromptDatasets/shared/__tests__/guidelinesSection.test.tsx

Nice refactoring here.

* prompt example tables displaying correctly * saving new prompt examples works * we are running trials * actually use default relevant text id * backend tests * frontend tests * fix lint error

* add type to table and color code it * finish updates to activities index page * possible to create a new activity with an ai type * nav updates * snapshot updates * wip * wip before i mess up further * wip -- actually saving working * saving works and they populate correctly too! * clean up * get invalid relevant texts populated on the activity form * small edits * tests * fix lint errors * remove trailing whitespace * clean up * fixes * fix test * all refactors except service object * refactor gen ai stuff into a service object; tests still pass * finish specs * fix lint errors * jamie's requested revisions * other refactors * thomas' revisions * fixes * try again * datasets pages (#12473) * wip * get basic tables loading * lots of cleanup to do but UPLOADING WORKS * style index page * successfully handling errors for new datasets * modal upload looks much better * set up individual stem vault pages * split out files * frontend tests * fix lints and tests * backend tests * more lint fixes * import fix * make sure notes stuff works with migration in place * fix lint error * fix function to accommodate has_many stem_vaults * fix test time incongruity * handle loading state * css adjustments * more styling * update snapshots * one more snapshot * add task_type to dataset on json request save * remove trailing whitespace * individual dataset page (#12486) * wip * get basic tables loading * lots of cleanup to do but UPLOADING WORKS * style index page * successfully handling errors for new datasets * modal upload looks much better * set up individual stem vault pages * split out files * frontend tests * fix lints and tests * backend tests * more lint fixes * import fix * make sure notes stuff works with migration in place * fix lint error * fix function to accommodate has_many stem_vaults * fix test time incongruity * handle loading state * css adjustments * more styling * update snapshots * one more snapshot * add task_type to dataset on json request save * remove trailing whitespace * wip * partway down road of updating datatable but there may be a better way * trial table rendering * make it possible to edit dataset notes * fix lint errors * Revert "partway down road of updating datatable but there may be a better way" This reverts commit e1a31fc. * bad commit * reorg * more tests and reorg * remove invalid aria attribute * update interfaces * guideline creation and display is working * hiding and unhiding guidelines also works * backend updates to create and set default relevant texts * all relevant text flows working * tests and minor accessibility updates * more jest test updates * backend tests and minor css changes * fix lint errors * lint errors and scss fixes * styling tweaks * fix capitalization error * fix * Bs individual dataset change to use nested attributes (#12499) * Use nested attributes for dataset_relevant_text create and update * fix specs * Fix specs --------- Co-authored-by: Brendan Shean <[email protected]> * test updates and fixes * thomas suggestions --------- Co-authored-by: Brendan Shean <[email protected]> * trial runner page pt 1 (#12503) * wip * get basic tables loading * lots of cleanup to do but UPLOADING WORKS * style index page * successfully handling errors for new datasets * modal upload looks much better * set up individual stem vault pages * split out files * frontend tests * fix lints and tests * backend tests * more lint fixes * import fix * make sure notes stuff works with migration in place * fix lint error * fix function to accommodate has_many stem_vaults * fix test time incongruity * handle loading state * css adjustments * more styling * update snapshots * one more snapshot * add task_type to dataset on json request save * remove trailing whitespace * wip * partway down road of updating datatable but there may be a better way * trial table rendering * make it possible to edit dataset notes * fix lint errors * Revert "partway down road of updating datatable but there may be a better way" This reverts commit e1a31fc. * bad commit * reorg * more tests and reorg * remove invalid aria attribute * update interfaces * guideline creation and display is working * hiding and unhiding guidelines also works * backend updates to create and set default relevant texts * all relevant text flows working * tests and minor accessibility updates * more jest test updates * backend tests and minor css changes * fix lint errors * lint errors and scss fixes * styling tweaks * fix capitalization error * fix * wip * wip * Bs individual dataset change to use nested attributes (#12499) * Use nested attributes for dataset_relevant_text create and update * fix specs * Fix specs --------- Co-authored-by: Brendan Shean <[email protected]> * test updates and fixes * wip and qa * making new llm prompt modal pretty * creating new prompt template variables works * view works and so does new template creation * make setters work * frontend tests * rspec tests * lint and test fixes * lint fix * remove duplicated line --------- Co-authored-by: Brendan Shean <[email protected]> * trial runner page pt 2 (#12517) * move stuff around and refactor guideline keys * add show test to dataset controller spec * updating queries to make cache invalidation possible * update tests * lint fixes * checkability for guidelines * update dataset relevant texts and guidelines to handle selection * tests * thomas suggestions * trial runner page pt 3 (#12518) * prompt example tables displaying correctly * saving new prompt examples works * we are running trials * actually use default relevant text id * backend tests * frontend tests * fix lint error * individual trial page part one (#12528) * move stuff around and refactor guideline keys * add show test to dataset controller spec * updating queries to make cache invalidation possible * update tests * lint fixes * checkability for guidelines * update dataset relevant texts and guidelines to handle selection * tests * prompt example tables displaying correctly * saving new prompt examples works * we are running trials * actually use default relevant text id * thomas suggestions * backend tests * frontend tests * fix lint error * wip * qa fixes * add modals and histogram * add confusion matrix * frontend tests * backend tests * lints and one test fix * minor scss updates * fix snapshot * fix * test updates * some width updates * brendan's suggestion * individual trial page pt 2 (#12531) * hacked but working * functionality good, but still routing weirdness * add comments for routes that aren't working * frontend tests * backend tests * lint/test fixes * test fixes and hopefully some minor speed gain * test edits * data subset page (#12535) * update the individualdataset and related components to handle subsets * additional minor adjustment * tests etc * oops * fix test and lint * trial comparisons page (#12543) * everything but individual trial header looks good * got table headers rendering all the relevant information * fix logic around colors * test updates * refactor out some more shared constants * update datatable and related tests * frontend tests * lint fixes * backend tests * fix lint * remove stray punctuation mark * fix canvas integration form tests * eric-suggested changes * fix broken test * fix lint error * some qa updates for evidence cms (#12569) * qa updates * tests * fix one other frontend test * brendan suggestions * llm prompt templates page (#12582) * qa updates * tests * fix one other frontend test * wip -- getting pieces in place * move snapshot * bulk update is working * frontend is styled and looking good * update param name * backend tests * frontend tests * minor width adjustments * minor scss update * lint fixes * adjust timestamp so tests pass * revision suggestions * adjust checkbox scss * lint fixes * negated matchers --------- Co-authored-by: Brendan Shean <[email protected]>

emilia-friedberg added 5 commits November 1, 2024 10:21

move stuff around and refactor guideline keys

7359c5b

add show test to dataset controller spec

c558f19

updating queries to make cache invalidation possible

8642adf

update tests

5841a57

lint fixes

f3210f3

emilia-friedberg changed the base branch from develop to feat/update-evidence-cms-to-support-activity-types November 1, 2024 16:00

emilia-friedberg added 3 commits November 1, 2024 12:34

checkability for guidelines

f5554d9

update dataset relevant texts and guidelines to handle selection

b1faff3

tests

2610e78

emilia-friedberg marked this pull request as ready for review November 1, 2024 17:48

emilia-friedberg requested review from anathomical, eadams17 and brendanshean November 1, 2024 17:48

emilia-friedberg commented Nov 1, 2024

View reviewed changes

anathomical approved these changes Nov 1, 2024

View reviewed changes

eadams17 approved these changes Nov 4, 2024

View reviewed changes

thomas suggestions

dd3c127

brendanshean approved these changes Nov 4, 2024

View reviewed changes

trial runner page pt 3 (#12518)

4c41046

* prompt example tables displaying correctly * saving new prompt examples works * we are running trials * actually use default relevant text id * backend tests * frontend tests * fix lint error

emilia-friedberg merged commit 3bed97a into feat/update-evidence-cms-to-support-activity-types Nov 7, 2024
1 of 2 checks passed

emilia-friedberg deleted the feat/trial-runner-page-pt-2 branch November 7, 2024 13:11

emilia-friedberg mentioned this pull request Dec 5, 2024

recreate genai trial runner and all associated pages in evidence cms #12632

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

trial runner page pt 2 #12517

trial runner page pt 2 #12517

emilia-friedberg commented Nov 1, 2024 •

edited

Loading

emilia-friedberg Nov 1, 2024

anathomical left a comment

anathomical Nov 1, 2024

anathomical Nov 1, 2024

anathomical Nov 1, 2024

anathomical Nov 1, 2024

anathomical Nov 1, 2024

eadams17 left a comment

brendanshean left a comment

brendanshean Nov 4, 2024

brendanshean Nov 4, 2024

trial runner page pt 2 #12517

trial runner page pt 2 #12517

Conversation

emilia-friedberg commented Nov 1, 2024 • edited Loading

WHAT

WHY

HOW

Screenshots

Notion Card Links

What have you done to QA this feature?

Choose a reason for hiding this comment

anathomical left a comment

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eadams17 left a comment

Choose a reason for hiding this comment

brendanshean left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

emilia-friedberg commented Nov 1, 2024 •

edited

Loading