Skip to content
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

fix(frontend): moved testset out of application scope #2111

Merged
merged 6 commits into from
Oct 15, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion agenta-web/cypress/e2e/eval.evaluations.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ describe("Evaluations CRUD Operations Test", function () {

context("Executing Evaluation with different answer column", () => {
it("Should successfully rename the testset columns", () => {
cy.visit(`/apps/${app_id}/testsets`)
cy.visit(`/apps/testsets`)
cy.location("pathname").should("include", "/testsets")
cy.get(".ant-table-row").eq(0).click()
cy.wait(1000)
Expand Down
2 changes: 1 addition & 1 deletion agenta-web/cypress/e2e/single-model-test-evaluation.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ describe("Single Model Test workflow", () => {
})

it("Should check the evaluation testset is successfully saved", () => {
cy.visit(`/apps/${app_id}/testsets`)
cy.visit(`/apps/testsets`)
cy.url().should("include", "/testsets")
cy.get('[data-cy="app-testset-list"]').as("table")
cy.get("@table").contains(saved_testset_name).as("tempTestSet").should("be.visible")
Expand Down
8 changes: 2 additions & 6 deletions agenta-web/cypress/e2e/testset.cy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,14 @@ const countries = [
]

describe("Testsets crud and UI functionality", () => {
let app_id
before(() => {
cy.createVariant()
cy.get("@app_id").then((appId) => {
app_id = appId
})
})

context("Testing creation process of testset", () => {
beforeEach(() => {
// navigate to the new testset page
cy.visit(`/apps/${app_id}/testsets`)
cy.visit(`/apps/testsets`)
})

it("Should successfully creates the testset and navigates to the list", () => {
Expand Down Expand Up @@ -56,7 +52,7 @@ describe("Testsets crud and UI functionality", () => {
context("When uploading testset", () => {
const testset_name = randString(8)
beforeEach(() => {
cy.visit(`/apps/${app_id}/testsets`)
cy.visit(`/apps/testsets`)
})

it("Should successfully upload a testset", () => {
Expand Down
3 changes: 2 additions & 1 deletion agenta-web/cypress/support/commands/evaluations.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,8 @@ Cypress.Commands.add("createVariant", () => {
Cypress.Commands.add("createVariantsAndTestsets", () => {
cy.createVariant()

cy.clickLinkAndWait('[data-cy="app-testsets-link"]')
cy.visit("/apps/testsets")
cy.url().should("include", "/testsets")
cy.get('[data-cy="create-testset-modal-button"]').click()
cy.get(".ant-modal-content").should("exist")
cy.get('[data-cy="create-testset-from-scratch"]').click()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -351,9 +351,7 @@ const AbTestingEvaluation = ({viewType}: {viewType: "evaluation" | "overview"})
icon: <Database size={16} />,
onClick: (e) => {
e.domEvent.stopPropagation()
router.push(
`/apps/${appId}/testsets/${record.testset._id}`,
)
router.push(`/apps/testsets/${record.testset._id}`)
},
},
{type: "divider"},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -275,7 +275,7 @@ const SingleModelEvaluation = ({viewType}: {viewType: "evaluation" | "overview"}
icon: <Database size={16} />,
onClick: (e) => {
e.domEvent.stopPropagation()
router.push(`/apps/${appId}/testsets/${record.testset._id}`)
router.push(`/apps/testsets/${record.testset._id}`)
},
},
{type: "divider"},
Expand Down
24 changes: 12 additions & 12 deletions agenta-web/src/components/Sidebar/config.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import {
Desktop,
GithubLogo,
PaperPlane,
PersonSimpleRun,
Phone,
Question,
Scroll,
Expand Down Expand Up @@ -40,7 +39,7 @@ export type SidebarConfig = {
export const useSidebarConfig = () => {
const appId = useAppId()
const {doesSessionExist} = useSession()
const {currentApp, recentlyVisitedAppId} = useAppsData()
const {currentApp, recentlyVisitedAppId, apps} = useAppsData()
const capitalizedAppName = renameVariablesCapitalizeAll(currentApp?.app_name || "")
const isOss = !isDemo()
const [useOrgData, setUseOrgData] = useState<Function>(() => () => "")
Expand All @@ -55,12 +54,21 @@ export const useSidebarConfig = () => {

const sidebarConfig: SidebarConfig[] = [
{
key: "app-management-link",
title: "App Management",
key: "application-link",
title: "Applications",
tooltip: "Create new applications or switch between your existing projects.",
link: "/apps",
icon: <AppstoreOutlined />,
divider: apps.length === 0 ? true : false,
},
{
key: "app-testsets-link",
title: "Test Sets",
tooltip: "Create and manage testsets for evaluation purposes.",
link: `/apps/testsets`,
icon: <DatabaseOutlined />,
divider: true,
isHidden: apps.length === 0,
},
{
key: `${currentApp?.app_name || ""}_key`,
Expand All @@ -84,14 +92,6 @@ export const useSidebarConfig = () => {
icon: <RocketOutlined />,
isHidden: !appId && !recentlyVisitedAppId,
},
{
key: "app-testsets-link",
title: "Test Sets",
tooltip: "Create and manage testsets for evaluation purposes.",
link: `/apps/${appId || recentlyVisitedAppId}/testsets`,
icon: <DatabaseOutlined />,
isHidden: !appId && !recentlyVisitedAppId,
},
{
key: "app-evaluations-link",
title: "Evaluations",
Expand Down
12 changes: 11 additions & 1 deletion agenta-web/src/components/TestSetTable/TestsetTable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import {NoticeType} from "antd/es/message/interface"
import {GenericObject, KeyValuePair} from "@/lib/Types"
import TableCellsRenderer from "./TableCellsRenderer"
import TableHeaderComponent from "./TableHeaderComponent"
import {useAppsData} from "@/contexts/app.context"

type TestsetTableProps = {
mode: "edit"
Expand Down Expand Up @@ -91,7 +92,9 @@ const TestsetTable: React.FC<TestsetTableProps> = ({mode}) => {
const router = useRouter()
const {appTheme} = useAppTheme()

const appId = router.query.app_id as string
const {apps, isLoading: isAppsLoading} = useAppsData()

const appId = apps[0]?.app_id
const {testset_id} = router.query

useBlockNavigation(unSavedChanges, {
Expand All @@ -112,6 +115,13 @@ const TestsetTable: React.FC<TestsetTableProps> = ({mode}) => {
}
}, [rowData, testsetName, columnDefs, inputValues])

useUpdateEffect(() => {
if ((apps.length === 0 || !apps) && !isAppsLoading) {
message.warning("To view the test set, you first need to create an app.")
router.push("/apps")
}
}, [isAppsLoading])

useEffect(() => {
async function applyColData(colData: {field: string}[] = []) {
const newColDefs = createNewColDefs(colData)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -425,7 +425,7 @@ const AutoEvaluation = () => {
icon: <Database size={16} />,
onClick: (e) => {
e.domEvent.stopPropagation()
router.push(`/apps/${appId}/testsets/${record.testset.id}`)
router.push(`/apps/testsets/${record.testset.id}`)
},
},
{type: "divider"},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -458,7 +458,7 @@ const EvaluationCompareMode: React.FC<Props> = () => {
<Space size="large">
<Space>
<Typography.Text strong>Testset:</Typography.Text>
<Typography.Link href={`/apps/${appId}/testsets/${testset?.id}`}>
<Typography.Link href={`/apps/testsets/${testset?.id}`}>
{testset?.name || ""}
</Typography.Link>
</Space>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -349,7 +349,7 @@ const EvaluationScenarios: React.FC<Props> = () => {
</Typography.Text>
<Space>
<Typography.Text strong>Testset:</Typography.Text>
<Typography.Link href={`/apps/${appId}/testsets/${evalaution?.testset.id}`}>
<Typography.Link href={`/apps/testsets/${evalaution?.testset.id}`}>
{evalaution?.testset.name || ""}
</Typography.Link>
</Space>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -370,7 +370,7 @@ const AutomaticEvalOverview = () => {
icon: <Database size={16} />,
onClick: (e) => {
e.domEvent.stopPropagation()
router.push(`/apps/${appId}/testsets/${record.testset.id}`)
router.push(`/apps/testsets/${record.testset.id}`)
},
},
{type: "divider"},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ const useStyles = createUseStyles((theme: JSSTheme) => ({
type Props = {
setCurrent: React.Dispatch<React.SetStateAction<number>>
onCancel: () => void
appId: string
}
type LanguageCodeBlockProps = {
selectedLang: string
Expand All @@ -67,14 +68,12 @@ const LanguageCodeBlock = ({selectedLang, codeSnippets}: LanguageCodeBlockProps)
)
}

const CreateTestsetFromApi: React.FC<Props> = ({setCurrent, onCancel}) => {
const CreateTestsetFromApi: React.FC<Props> = ({setCurrent, onCancel, appId}) => {
const classes = useStyles()
const router = useRouter()
const [uploadType, setUploadType] = useState<"csv" | "json">("csv")
const [selectedLang, setSelectedLang] = useState("python")

const appId = router.query.app_id as string

const uploadURI = `${getAgentaApiUrl()}/api/testsets/upload`
const jsonURI = `${getAgentaApiUrl()}/api/testsets/${appId}`

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ type Props = {
setEditTestsetValues: React.Dispatch<React.SetStateAction<testset | null>>
setCurrent: React.Dispatch<React.SetStateAction<number>>
onCancel: () => void
appId: string
}

const CreateTestsetFromScratch: React.FC<Props> = ({
Expand All @@ -42,10 +43,10 @@ const CreateTestsetFromScratch: React.FC<Props> = ({
setEditTestsetValues,
setCurrent,
onCancel,
appId,
}) => {
const classes = useStyles()
const router = useRouter()
const appId = router.query.app_id as string
const [testsetName, setTestsetName] = useState(
mode === "rename" ? (editTestsetValues?.name as string) : "",
)
Expand All @@ -68,7 +69,7 @@ const CreateTestsetFromScratch: React.FC<Props> = ({
const rowData = data || (await generateInitialRowData())
const response = await createNewTestset(appId, testsetName, rowData)
message.success("Test set created successfully")
router.push(`/apps/${appId}/testsets/${response.data.id}`)
router.push(`/apps/testsets/${response.data.id}`)
} catch (error) {
console.error("Error saving test set:", error)
message.error("Failed to create Test set. Please try again!")
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,14 +49,14 @@ const useStyles = createUseStyles((theme: JSSTheme) => ({
type Props = {
setCurrent: React.Dispatch<React.SetStateAction<number>>
onCancel: () => void
appId: string
}

const UploadTestset: React.FC<Props> = ({setCurrent, onCancel}) => {
const UploadTestset: React.FC<Props> = ({setCurrent, onCancel, appId}) => {
const classes = useStyles()
const router = useRouter()
const [form] = Form.useForm()
const testsetFile = Form.useWatch("file", form)
const appId = router.query.app_id as string
const [uploadType, setUploadType] = useState<"JSON" | "CSV" | undefined>("CSV")
const [testsetName, setTestsetName] = useState("")
const [uploadLoading, setUploadLoading] = useState(false)
Expand Down
9 changes: 7 additions & 2 deletions agenta-web/src/components/pages/testset/modals/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ type Props = {
setEditTestsetValues: React.Dispatch<React.SetStateAction<testset | null>>
current: number
setCurrent: React.Dispatch<React.SetStateAction<number>>
appId: string
} & React.ComponentProps<typeof Modal>

const TestsetModal: React.FC<Props> = ({
Expand All @@ -36,6 +37,7 @@ const TestsetModal: React.FC<Props> = ({
setEditTestsetValues,
current,
setCurrent,
appId,
...props
}) => {
const classes = useStyles()
Expand All @@ -61,14 +63,17 @@ const TestsetModal: React.FC<Props> = ({
onCancel={onCancel}
editTestsetValues={editTestsetValues}
setEditTestsetValues={setEditTestsetValues}
appId={appId}
/>
),
},
{
content: <UploadTestset setCurrent={setCurrent} onCancel={onCancel} />,
content: <UploadTestset setCurrent={setCurrent} onCancel={onCancel} appId={appId} />,
},
{
content: <CreateTestsetFromApi setCurrent={setCurrent} onCancel={onCancel} />,
content: (
<CreateTestsetFromApi setCurrent={setCurrent} onCancel={onCancel} appId={appId} />
),
},
]

Expand Down
8 changes: 7 additions & 1 deletion agenta-web/src/lib/helpers/evaluate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -239,7 +239,13 @@ export const getVotesPercentage = (record: HumanEvaluationListTableDataType, ind
export const checkIfResourceValidForDeletion = async (
data: Omit<Parameters<typeof fetchEvaluatonIdsByResource>[0], "appId">,
) => {
const appId = getAppValues().currentApp?.app_id
let appId

if (data.resourceType === "testset") {
appId = getAppValues().apps[0]?.app_id
} else {
appId = getAppValues().currentApp?.app_id
}
if (!appId) return false

const response = await fetchEvaluatonIdsByResource({...data, appId})
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,11 +7,13 @@ import {JSSTheme, TestSet, testset, TestsetCreationMode} from "@/lib/Types"
import {deleteTestsets, useLoadTestsetsList} from "@/services/testsets/api"
import {MoreOutlined, PlusOutlined} from "@ant-design/icons"
import {Copy, GearSix, Note, PencilSimple, Trash} from "@phosphor-icons/react"
import {Button, Dropdown, Input, Spin, Table, Typography} from "antd"
import {Button, Dropdown, Input, message, Spin, Table, Typography} from "antd"
import {ColumnsType} from "antd/es/table/interface"
import {useRouter} from "next/router"
import {createUseStyles} from "react-jss"
import dayjs from "dayjs"
import {useUpdateEffect} from "usehooks-ts"
import {useAppsData} from "@/contexts/app.context"

const useStyles = createUseStyles((theme: JSSTheme) => ({
modal: {
Expand Down Expand Up @@ -53,7 +55,8 @@ const useStyles = createUseStyles((theme: JSSTheme) => ({
const Testset = () => {
const classes = useStyles()
const router = useRouter()
const appId = router.query.app_id as string
const {apps, isLoading: isAppsLoading} = useAppsData()
const appId = apps[0]?.app_id
const [selectedRowKeys, setSelectedRowKeys] = useState<React.Key[]>([])
const {testsets, isTestsetsLoading, mutate} = useLoadTestsetsList(appId)
const [isCreateTestsetModalOpen, setIsCreateTestsetModalOpen] = useState(false)
Expand All @@ -62,6 +65,13 @@ const Testset = () => {
const [editTestsetValues, setEditTestsetValues] = useState<testset | null>(null)
const [current, setCurrent] = useState(0)

useUpdateEffect(() => {
if ((apps.length === 0 || !apps) && !isAppsLoading) {
message.warning("To view the test set, you first need to create an app.")
router.push("/apps")
}
}, [isAppsLoading])

const rowSelection = {
onChange: (selectedRowKeys: React.Key[]) => {
setSelectedRowKeys(selectedRowKeys)
Expand Down Expand Up @@ -147,7 +157,7 @@ const Testset = () => {
icon: <Note size={16} />,
onClick: (e) => {
e.domEvent.stopPropagation()
router.push(`/apps/${appId}/testsets/${record._id}`)
router.push(`/apps/testsets/${record._id}`)
},
},
{
Expand Down Expand Up @@ -247,12 +257,13 @@ const Testset = () => {
columns={columns}
dataSource={filteredTestset}
rowKey="_id"
loading={isTestsetsLoading}
loading={isTestsetsLoading || isAppsLoading}
scroll={{x: true}}
pagination={false}
onRow={(record) => {
return {
onClick: () => router.push(`/apps/${appId}/testsets/${record._id}`),
onClick: () => router.push(`/apps/testsets/${record._id}`),
style: {cursor: "pointer"},
}
}}
locale={{emptyText: <NoResultsFound />}}
Expand All @@ -267,6 +278,7 @@ const Testset = () => {
testsetCreationMode={testsetCreationMode}
setTestsetCreationMode={setTestsetCreationMode}
open={isCreateTestsetModalOpen}
appId={appId}
onCancel={() => {
setIsCreateTestsetModalOpen(false)
}}
Expand Down
Loading