Conversation
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughThis PR replaces dummy GraphQL documents with real operations: adds GET_ASSISTANT_CONFIG_VERSIONS, CREATE_EVALUATION, and LIST_AI_EVALUATIONS. AIEvaluationCreate now queries assistant config versions to populate the assistant dropdown, uses a CREATE_EVALUATION mutation with success/error handlers and navigation, updates form payload construction to derive configId from selected assistant version, and changes Golden QA state to include datasetId and name (goldenQaId becomes numeric). UploadGoldenQaDialog.onProceed, tests, and mocks were updated to include datasetId. CSS tweaks adjust form layout alignment and Golden QA button styling. Several mocks were added/updated to reflect new queries/mutation shapes. Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
🚀 Deployed on https://deploy-preview-3844--glific-frontend.netlify.app |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/containers/AIEvals/AIEvaluationCreate/AIEvaluationCreate.test.tsx (1)
195-197: Avoid passinggetAssistantConfigVersionsMocktwice in this test setup.
defaultMocksalready includes this mock viagetAIEvaluationCreateMocks, so appending it again is redundant and can make mock resolution less deterministic.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/containers/AIEvals/AIEvaluationCreate/AIEvaluationCreate.test.tsx` around lines 195 - 197, The test 'shows assistant options from query using assistantName and versionNumber' is passing getAssistantConfigVersionsMock twice by appending it to defaultMocks; remove the duplicate so mocks aren’t duplicated. Update the render call in the test to use only defaultMocks (which already includes getAssistantConfigVersionsMock via getAIEvaluationCreateMocks) instead of [...defaultMocks, getAssistantConfigVersionsMock], ensuring the test uses the single canonical mock instance.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/containers/AIEvals/AIEvaluationCreate/AIEvaluationCreate.tsx`:
- Around line 161-166: In the SectionDivider field config for
__evaluationDetailsDivider, fix the misspelled key "placedolder" to
"placeholder" so the placeholder behavior is applied; update the object property
name in the AIEvaluationCreate component where SectionDivider is defined (look
for the __evaluationDetailsDivider config) to use "placeholder" with the
intended string value.
- Around line 41-42: The new UI strings in AIEvaluationCreate.tsx are
hard-coded; import and use the i18next hook (useTranslation) and replace each
literal (the paragraph around "Select The Golden QA Dataset..." plus the other
literals at the noted locations 46-50, 68, 172) with t('your.namespace.key')
calls (e.g., t('aiEvaluation.selectGoldenDataset')) and add corresponding keys
to the locale files so they are picked up by yarn extract-translations; update
the AIEvaluationCreate component to call const { t } = useTranslation() and use
t(...) for all visible strings so localization coverage is preserved.
In `@src/mocks/AIEvaluations.ts`:
- Around line 14-42: The mock getAssistantConfigVersionsMock is missing the
kaapiUuid field which GET_ASSISTANT_CONFIG_VERSIONS queries; update each
assistantConfigVersions item in getAssistantConfigVersionsMock to include a
kaapiUuid value (e.g., a unique string) so the mock shape matches the GraphQL
selection set and prevents missing-field cache warnings.
---
Nitpick comments:
In `@src/containers/AIEvals/AIEvaluationCreate/AIEvaluationCreate.test.tsx`:
- Around line 195-197: The test 'shows assistant options from query using
assistantName and versionNumber' is passing getAssistantConfigVersionsMock twice
by appending it to defaultMocks; remove the duplicate so mocks aren’t
duplicated. Update the render call in the test to use only defaultMocks (which
already includes getAssistantConfigVersionsMock via getAIEvaluationCreateMocks)
instead of [...defaultMocks, getAssistantConfigVersionsMock], ensuring the test
uses the single canonical mock instance.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 2469974e-ccb4-4210-9b57-7f6627e7573c
📒 Files selected for processing (6)
src/containers/AIEvals/AIEvaluationCreate/AIEvaluationCreate.module.csssrc/containers/AIEvals/AIEvaluationCreate/AIEvaluationCreate.test.tsxsrc/containers/AIEvals/AIEvaluationCreate/AIEvaluationCreate.tsxsrc/graphql/mutations/AIEvaluations.tssrc/graphql/queries/Assistant.tssrc/mocks/AIEvaluations.ts
src/containers/AIEvals/AIEvaluationCreate/AIEvaluationCreate.tsx
Outdated
Show resolved
Hide resolved
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #3844 +/- ##
==========================================
+ Coverage 81.88% 81.91% +0.02%
==========================================
Files 308 309 +1
Lines 12940 12953 +13
Branches 2978 2982 +4
==========================================
+ Hits 10596 10610 +14
Misses 1409 1409
+ Partials 935 934 -1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Glific
|
||||||||||||||||||||||||||||
| Project |
Glific
|
| Branch Review |
feat/fix_create_evalution_page
|
| Run status |
|
| Run duration | 08m 36s |
| Commit |
|
| Committer | Priyanshu singh |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
0
|
Flaky
|
0
|
Pending
|
0
|
Skipped
|
0
|
Passing
|
62
|
| View all changes introduced in this branch ↗︎ | |
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/containers/AIEvals/AIEvaluationCreate/UploadGoldenQaDialog.tsx (1)
90-100:⚠️ Potential issue | 🟠 MajorValidate parsed
datasetIdbefore callingonProceed.The backend returns
datasetIdas a string, andparseInt()at line 97 can returnNaNif the value is invalid or non-numeric. The current guard at line 90 only checks forgoldenQaandgoldenQa.nameexistence, allowing invaliddatasetIdvalues to propagate to the callback, which expects anumbertype.Suggested patch
- if (!goldenQa || !goldenQa.name) { + const parsedDatasetId = Number(goldenQa?.datasetId); + if (!goldenQa || !goldenQa.name || !Number.isInteger(parsedDatasetId)) { setNotification('Failed to upload Golden QA', 'error'); return; } setNotification('Golden QA uploaded successfully', 'success'); onProceed({ - datasetId: parseInt(goldenQa.datasetId, 10), + datasetId: parsedDatasetId, name: goldenQa.name, duplicationFactor: goldenQa.duplication_factor, });🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/containers/AIEvals/AIEvaluationCreate/UploadGoldenQaDialog.tsx` around lines 90 - 100, The parsed datasetId from goldenQa.datasetId must be validated before calling onProceed: in UploadGoldenQaDialog, parse goldenQa.datasetId with parseInt into a variable (e.g., parsedDatasetId), check Number.isFinite(parsedDatasetId) or !Number.isNaN(parsedDatasetId) and that it is > 0 (or meets your domain rules); if invalid, call setNotification('Invalid dataset ID in uploaded Golden QA', 'error') and return instead of invoking onProceed; only call onProceed({ datasetId: parsedDatasetId, name: goldenQa.name, duplicationFactor: goldenQa.duplication_factor }) when the parsed value is valid.
♻️ Duplicate comments (2)
src/containers/AIEvals/AIEvaluationCreate/AIEvaluationCreate.tsx (2)
139-144:⚠️ Potential issue | 🟡 MinorFix typo in field config key (
placedolder→placeholder).Line 143 uses a misspelled key, so the config entry is ignored.
Suggested patch
{ component: SectionDivider, name: '__evaluationDetailsDivider', label: 'Evaluation Details', - placedolder: '', + placeholder: '', },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/containers/AIEvals/AIEvaluationCreate/AIEvaluationCreate.tsx` around lines 139 - 144, The field config for the SectionDivider entry named '__evaluationDetailsDivider' has a typo in its key ('placedolder') so the placeholder is ignored; update the config by renaming the key from 'placedolder' to 'placeholder' on the object with component SectionDivider and name '__evaluationDetailsDivider' so the placeholder value is applied correctly.
20-29:⚠️ Potential issue | 🟠 MajorUse i18next keys instead of hard-coded UI strings.
New/updated user-facing literals here should be wrapped with
t(...)to keep localization coverage intact.As per coding guidelines, "Use i18next for internationalization with string extraction to Lokalise via
yarn extract-translations".Also applies to: 67-68, 133-159, 198-200, 213-213
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/containers/AIEvals/AIEvaluationCreate/AIEvaluationCreate.tsx` around lines 20 - 29, In AIEvaluationCreate, replace all hard-coded UI strings (e.g., "Expected CSV Format:", "Question, Answer", '{"What Is X"},{"Answer"}', "Click Here For The Template Csv" and other literals at the referenced ranges) with i18next calls using the useTranslation hook (e.g., const { t } = useTranslation()) and t('your.key.here') keys, add appropriate translation keys (camel/snake as project convention) and update JSX to use t(...) for labels and link text; after changes run the translation extraction step (yarn extract-translations) to ensure keys are picked up and include keys for the other mentioned sections (lines ~67-68, 133-159, 198-200, 213) as well.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/containers/AIEvals/AIEvaluationCreate/AIEvaluationCreate.tsx`:
- Around line 75-78: goldenQaOptions uses mixed id types causing goldenQaId
(typed number) to accept the placeholder; make ids consistent and validate as a
number: change the placeholder option id from '0' to '' in goldenQaOptions (so
the select shows no valid id), update the validation schema that checks
goldenQaId to require a number with min(1) (or equivalent numeric minimum)
instead of a string check, and when building the submission payload convert the
selected id to a numeric datasetId (e.g., Number(goldenQaId)) or handle the
empty string as absent; update references to goldenQADatasets.map,
goldenQaOptions, goldenQaId state, and the form validation schema accordingly.
In `@src/graphql/mutations/AIEvaluations.ts`:
- Around line 13-17: The CREATE_GOLDEN_QA GraphQL mutation's selection set is
missing the duplication_factor field, so goldenQa.duplication_factor is
undefined when UploadGoldenQaDialog reads it; update the createGoldenQa
selection (the goldenQa { ... } block in the CREATE_GOLDEN_QA mutation) to
include duplication_factor so Apollo fetches that value for
UploadGoldenQaDialog.
---
Outside diff comments:
In `@src/containers/AIEvals/AIEvaluationCreate/UploadGoldenQaDialog.tsx`:
- Around line 90-100: The parsed datasetId from goldenQa.datasetId must be
validated before calling onProceed: in UploadGoldenQaDialog, parse
goldenQa.datasetId with parseInt into a variable (e.g., parsedDatasetId), check
Number.isFinite(parsedDatasetId) or !Number.isNaN(parsedDatasetId) and that it
is > 0 (or meets your domain rules); if invalid, call setNotification('Invalid
dataset ID in uploaded Golden QA', 'error') and return instead of invoking
onProceed; only call onProceed({ datasetId: parsedDatasetId, name:
goldenQa.name, duplicationFactor: goldenQa.duplication_factor }) when the parsed
value is valid.
---
Duplicate comments:
In `@src/containers/AIEvals/AIEvaluationCreate/AIEvaluationCreate.tsx`:
- Around line 139-144: The field config for the SectionDivider entry named
'__evaluationDetailsDivider' has a typo in its key ('placedolder') so the
placeholder is ignored; update the config by renaming the key from 'placedolder'
to 'placeholder' on the object with component SectionDivider and name
'__evaluationDetailsDivider' so the placeholder value is applied correctly.
- Around line 20-29: In AIEvaluationCreate, replace all hard-coded UI strings
(e.g., "Expected CSV Format:", "Question, Answer", '{"What Is X"},{"Answer"}',
"Click Here For The Template Csv" and other literals at the referenced ranges)
with i18next calls using the useTranslation hook (e.g., const { t } =
useTranslation()) and t('your.key.here') keys, add appropriate translation keys
(camel/snake as project convention) and update JSX to use t(...) for labels and
link text; after changes run the translation extraction step (yarn
extract-translations) to ensure keys are picked up and include keys for the
other mentioned sections (lines ~67-68, 133-159, 198-200, 213) as well.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: ec368d63-aa4b-46f6-854b-30200aa44783
📒 Files selected for processing (8)
src/containers/AIEvals/AIEvaluationCreate/AIEvaluationCreate.module.csssrc/containers/AIEvals/AIEvaluationCreate/AIEvaluationCreate.test.tsxsrc/containers/AIEvals/AIEvaluationCreate/AIEvaluationCreate.tsxsrc/containers/AIEvals/AIEvaluationCreate/UploadGoldenQaDialog.test.tsxsrc/containers/AIEvals/AIEvaluationCreate/UploadGoldenQaDialog.tsxsrc/graphql/mutations/AIEvaluations.tssrc/graphql/queries/AIEvaluations.tssrc/mocks/AIEvaluations.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/containers/AIEvals/AIEvaluationCreate/AIEvaluationCreate.module.css
|
Note: Code coverage may fail since the UI is not fully ready yet. Hence, I’m not adding test cases for the create evaluation feature at this stage. I will include them in my test PR. |
src/containers/AIEvals/AIEvaluationCreate/AIEvaluationCreate.tsx
Outdated
Show resolved
Hide resolved
src/containers/AIEvals/AIEvaluationCreate/AIEvaluationCreate.test.tsx
Outdated
Show resolved
Hide resolved
src/containers/AIEvals/AIEvaluationCreate/AIEvaluationCreate.test.tsx
Outdated
Show resolved
Hide resolved
src/containers/AIEvals/AIEvaluationCreate/AIEvaluationCreate.tsx
Outdated
Show resolved
Hide resolved
src/containers/AIEvals/AIEvaluationCreate/AIEvaluationCreate.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (3)
src/containers/AIEvals/AIEvaluationCreate/AIEvaluationCreate.tsx (2)
79-81: Remove unnecessaryvariablesfrom query that accepts no arguments.The
GET_ASSISTANT_CONFIG_VERSIONSquery definition (insrc/graphql/queries/Assistant.ts) takes no variables, but this call passes{ filter: {} }. While Apollo may ignore extraneous variables, this creates misleading code and could fail if the server enforces strict variable validation.Suggested fix
- const { data: versionsData } = useQuery(GET_ASSISTANT_CONFIG_VERSIONS, { - variables: { filter: {} }, - }); + const { data: versionsData, loading: assistantsLoading } = useQuery(GET_ASSISTANT_CONFIG_VERSIONS);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/containers/AIEvals/AIEvaluationCreate/AIEvaluationCreate.tsx` around lines 79 - 81, The useQuery call passing unnecessary variables should be simplified: remove the variables object from the useQuery invocation that queries GET_ASSISTANT_CONFIG_VERSIONS in AIEvaluationCreate (the useQuery(GET_ASSISTANT_CONFIG_VERSIONS, { variables: { filter: {} } }) call) because the GraphQL operation accepts no variables; update the call to just useQuery(GET_ASSISTANT_CONFIG_VERSIONS) so extraneous variables aren’t sent and to avoid misleading future readers.
187-190: FormLayout query props are placeholders that could cause confusing behavior.
getItemQueryis set toGET_ASSISTANT_CONFIG_VERSIONS(fetches assistants, not evaluations), and all three mutation props useCREATE_EVALUATION. SincehandleSetPayloadtriggers the actual mutation, these props act as placeholders—but ifFormLayoutattempts to use them (e.g., for edit mode or delete), the behavior will be incorrect. Consider usingnullor creating proper no-op queries to avoid accidental misuse.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/containers/AIEvals/AIEvaluationCreate/AIEvaluationCreate.tsx` around lines 187 - 190, FormLayout's GraphQL props are using incorrect placeholders (getItemQuery=GET_ASSISTANT_CONFIG_VERSIONS and create/update/deleteItemQuery=CREATE_EVALUATION) which can cause wrong behavior if FormLayout ever invokes them; change those props to explicit null (or a dedicated no-op query/mutation) so they cannot be accidentally used, and leave actual mutation handling to handleSetPayload; update the JSX where FormLayout is instantiated (look for the AIEvaluationCreate component and the FormLayout prop assignments) to pass null or proper no-op operations for getItemQuery, createItemQuery, updateItemQuery, and deleteItemQuery.src/containers/AIEvals/AIEvaluationCreate/AIEvaluationCreate.test.tsx (1)
287-294: Mock usesvariableMatcher: () => true, which won't catch incorrect mutation variables.The
createGoldenQaCustomSuccessMock(persrc/mocks/AIEvaluations.ts) uses a catch-all variable matcher, so tests will pass even if the component sends incorrect or missing fields in the mutation. Consider adding a stricter mock for critical mutation tests to validate the exact payload structure.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/containers/AIEvals/AIEvaluationCreate/AIEvaluationCreate.test.tsx` around lines 287 - 294, The test relies on createGoldenQaCustomSuccessMock which currently uses variableMatcher: () => true, so update the mock in the test to use a stricter variable matcher that asserts the exact mutation payload (e.g., match the expected input fields like question, answer, metadata/id) or replace the catch-all mock with a new mock variant that validates variables; specifically modify createGoldenQaCustomSuccessMock (or add createGoldenQaStrictSuccessMock) to compare incoming variables to the expected GraphQL input shape and only return the success response when they match, then use that stricter mock in the failing test to ensure the component sends the correct mutation variables.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/containers/AIEvals/AIEvaluationCreate/AIEvaluationCreate.test.tsx`:
- Around line 166-177: The test 'accepts valid evaluation name with
alphanumeric, underscore and hyphen' asserts pattern acceptance but the
component's validation schema uses Yup.string().required() (no matches pattern),
so the test is misleading; either restore the pattern check in the component's
validation schema by adding .matches(/^[a-zA-Z0-9_-]+$/) to the field validation
in the AIEvaluationCreate form schema (the Yup validation used to validate the
name) OR rename/update the test to assert only that a non-empty name is accepted
(change the test title and expectations to reflect required-only validation),
and ensure any references to the name input (placeholder 'Give a unique name for
the evaluation experiment') remain correct.
- Line 14: The test creates duplicate Apollo mocks by appending
getAssistantConfigVersionsMock to the array returned by
getAIEvaluationCreateMocks(); update the test to stop adding
getAssistantConfigVersionsMock twice—use the defaultMocks array directly (e.g.,
use [...defaultMocks] or defaultMocks) instead of [...defaultMocks,
getAssistantConfigVersionsMock], removing the extra
getAssistantConfigVersionsMock to avoid duplicate mock entries (refer to
getAIEvaluationCreateMocks, getAssistantConfigVersionsMock, and defaultMocks).
In `@src/containers/AIEvals/AIEvaluationCreate/AIEvaluationCreate.tsx`:
- Around line 120-124: The handler handleUploadGoldenQaProceed currently accepts
{ datasetId, name } but UploadGoldenQaDialogProps.onProceed requires {
datasetId, name, duplicationFactor }; update handleUploadGoldenQaProceed to
accept duplicationFactor as well and include it where appropriate (e.g., add
duplicationFactor to the new item pushed into setGoldenQADatasets and/or include
it in setStates if golden QA state needs it), then keep closing behavior
setShowUploadGoldenQaDialog(false); ensuring the parameter shape matches the
onProceed type so TypeScript no longer errors.
- Around line 161-174: The payload builder in handleSetPayload is incorrectly
passing payload.assistantId as configVersion; change it to use the selected
version's versionNumber by setting configVersion to
selectedVersion?.versionNumber (from versionsData.assistantConfigVersions) when
calling createEvaluation in handleSetPayload so the backend receives the actual
versionNumber instead of the DB id.
---
Nitpick comments:
In `@src/containers/AIEvals/AIEvaluationCreate/AIEvaluationCreate.test.tsx`:
- Around line 287-294: The test relies on createGoldenQaCustomSuccessMock which
currently uses variableMatcher: () => true, so update the mock in the test to
use a stricter variable matcher that asserts the exact mutation payload (e.g.,
match the expected input fields like question, answer, metadata/id) or replace
the catch-all mock with a new mock variant that validates variables;
specifically modify createGoldenQaCustomSuccessMock (or add
createGoldenQaStrictSuccessMock) to compare incoming variables to the expected
GraphQL input shape and only return the success response when they match, then
use that stricter mock in the failing test to ensure the component sends the
correct mutation variables.
In `@src/containers/AIEvals/AIEvaluationCreate/AIEvaluationCreate.tsx`:
- Around line 79-81: The useQuery call passing unnecessary variables should be
simplified: remove the variables object from the useQuery invocation that
queries GET_ASSISTANT_CONFIG_VERSIONS in AIEvaluationCreate (the
useQuery(GET_ASSISTANT_CONFIG_VERSIONS, { variables: { filter: {} } }) call)
because the GraphQL operation accepts no variables; update the call to just
useQuery(GET_ASSISTANT_CONFIG_VERSIONS) so extraneous variables aren’t sent and
to avoid misleading future readers.
- Around line 187-190: FormLayout's GraphQL props are using incorrect
placeholders (getItemQuery=GET_ASSISTANT_CONFIG_VERSIONS and
create/update/deleteItemQuery=CREATE_EVALUATION) which can cause wrong behavior
if FormLayout ever invokes them; change those props to explicit null (or a
dedicated no-op query/mutation) so they cannot be accidentally used, and leave
actual mutation handling to handleSetPayload; update the JSX where FormLayout is
instantiated (look for the AIEvaluationCreate component and the FormLayout prop
assignments) to pass null or proper no-op operations for getItemQuery,
createItemQuery, updateItemQuery, and deleteItemQuery.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: c3462292-f4d5-43de-b5d9-e14b8fe17eae
📒 Files selected for processing (3)
src/containers/AIEvals/AIEvaluationCreate/AIEvaluationCreate.test.tsxsrc/containers/AIEvals/AIEvaluationCreate/AIEvaluationCreate.tsxsrc/mocks/AIEvaluations.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- src/mocks/AIEvaluations.ts
src/containers/AIEvals/AIEvaluationCreate/AIEvaluationCreate.test.tsx
Outdated
Show resolved
Hide resolved
src/containers/AIEvals/AIEvaluationCreate/AIEvaluationCreate.tsx
Outdated
Show resolved
Hide resolved
src/containers/AIEvals/AIEvaluationCreate/AIEvaluationCreate.tsx
Outdated
Show resolved
Hide resolved
src/containers/AIEvals/AIEvaluationCreate/AIEvaluationCreate.tsx
Outdated
Show resolved
Hide resolved
src/containers/AIEvals/AIEvaluationCreate/AIEvaluationCreate.tsx
Outdated
Show resolved
Hide resolved
src/containers/AIEvals/AIEvaluationCreate/AIEvaluationCreate.tsx
Outdated
Show resolved
Hide resolved
rvignesh89
left a comment
rvignesh89
left a comment
There was a problem hiding this comment.
Please address comments before merging.
Apart from the comments please also fix the following UX.
Current: When I enter evaluation name and select a version, and then go to upload golden qa, the selected fields are reset.
Expected: Selected field values should remain.
| expect(screen.getByText('Click here for the template CSV')).toBeInTheDocument(); | ||
| expect(screen.getByText('Question, Answer')).toBeInTheDocument(); | ||
| expect(screen.getByText('{"What Is X"},{"Answer"}')).toBeInTheDocument(); | ||
| expect(screen.getByText('Click Here For The Template Csv')).toBeInTheDocument(); |
There was a problem hiding this comment.
@priyanshu6238 why do these values keep changing?
There was a problem hiding this comment.
i change it according to the noopur design
|
|
||
| useEffect(() => { | ||
| if (versionsError) { | ||
| setNotification(versionsError.message, 'warning'); |
There was a problem hiding this comment.
This message should be prepended to say failed to fetch assistant versions. Otherwise user has no idea where the failure occurred.
There was a problem hiding this comment.
This needs to be handled on the backend. I will include a general message for this in my next backend PR.
|
Making it draft will pick it again in next iteration. when we again start the evals work |
2 participants
Summary
backend pr -glific/glific#4905
Summary by CodeRabbit
New Features
Improvements