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 batch range line selector and also allow to create logs on draft commits #202

Merged
merged 1 commit into from
Sep 18, 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: 2 additions & 0 deletions apps/web/src/actions/evaluations/runBatch.ts
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,8 @@ export const runBatchEvaluationAction = withDataset
evaluation,
dataset: ctx.dataset,
document: ctx.document,
projectId: ctx.project.id,
commitUuid: ctx.currentCommitUuid,
fromLine: input.fromLine,
toLine: input.toLine,
parametersMap: input.parameters,
Expand Down
2 changes: 1 addition & 1 deletion apps/web/src/actions/procedures/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,5 +54,5 @@ export const widthDocument = createServerActionProcedure(withProject)
})
.then((r) => r.unwrap())

return { ...ctx, document }
return { ...ctx, document, currentCommitUuid: input.commitUuid }
})
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,7 @@ export default function DocumentEditor({
}).then(setMetadata)
}, [readDocument])

const isMerged = commit.mergedAt !== null
return (
<DocumentEditorContext.Provider
value={{
Expand All @@ -130,6 +131,7 @@ export default function DocumentEditor({
<div className='flex flex-row w-full h-full gap-8 p-6'>
<div className='flex flex-col flex-1 flex-grow flex-shrink gap-2 min-w-0'>
<EditorHeader
disabledMetadataSelectors={isMerged}
title='Prompt editor'
metadata={metadata}
prompt={value}
Expand All @@ -141,9 +143,7 @@ export default function DocumentEditor({
metadata={metadata}
onChange={onChange}
readOnlyMessage={
commit.mergedAt
? 'Create a draft to edit documents.'
: undefined
isMerged ? 'Create a draft to edit documents.' : undefined
}
isSaved={isSaved}
/>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,7 @@ export function BigNumberPanels({
<div className='flex flex-wrap gap-6'>
<Panel label='Total logs' value={String(evaluationResults.length)} />
<Panel label='Total cost' value={formatCostInMillicents(totalCost)} />
<Panel label='Total cost' value={String(totalTokens)} />
<Panel label='Total tokens' value={String(totalTokens)} />
<TypeSpecificPanel
evaluation={evaluation}
evaluationResults={evaluationResults}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,10 +1,11 @@
import { useMemo, useState } from 'react'
import { useMemo } from 'react'

import { Dataset } from '@latitude-data/core/browser'
import {
FormFieldGroup,
Input,
NumeredList,
ReactStateDispatch,
Select,
SelectOption,
SwitchInput,
Expand All @@ -14,14 +15,15 @@ function LineRangeInputs({
disabled,
fromDefaultValue,
toDefaultValue,
onChangeToLine,
max,
}: {
disabled: boolean
fromDefaultValue: number | undefined
toDefaultValue: number | undefined
onChangeToLine: ReactStateDispatch<number | undefined>
max: number | undefined
}) {
const [to, setTo] = useState(toDefaultValue)
return (
<FormFieldGroup>
<Input
Expand All @@ -31,8 +33,8 @@ function LineRangeInputs({
label='From line'
defaultValue={fromDefaultValue}
placeholder='Starting line'
min={0}
max={to}
min={1}
max={toDefaultValue}
/>
<Input
disabled={disabled}
Expand All @@ -41,8 +43,10 @@ function LineRangeInputs({
label='To line'
placeholder='Ending line'
defaultValue={toDefaultValue}
onChange={(e) => setTo(Number(e.target.value))}
min={0}
onChange={(e) => {
onChangeToLine(Number(e.target.value))
}}
min={fromDefaultValue}
max={max}
/>
</FormFieldGroup>
Expand All @@ -56,6 +60,7 @@ export default function DatasetForm({
wantAllLines,
fromLine,
toLine,
onChangeToLine,
datasets,
isLoadingDatasets,
parametersList,
Expand All @@ -68,6 +73,7 @@ export default function DatasetForm({
wantAllLines: boolean
fromLine: number | undefined
toLine: number | undefined
onChangeToLine: ReactStateDispatch<number | undefined>
headers: SelectOption[]
selectedDataset: Dataset | null
datasets: Dataset[]
Expand Down Expand Up @@ -122,6 +128,7 @@ export default function DatasetForm({
disabled={wantAllLines}
fromDefaultValue={fromLine}
toDefaultValue={toLine}
onChangeToLine={onChangeToLine}
max={selectedDataset?.fileMetadata?.rowCount}
/>
<SwitchInput
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,6 +94,7 @@ export default function CreateBatchEvaluationModal({
wantAllLines={form.wantAllLines}
fromLine={form.fromLine}
toLine={form.toLine}
onChangeToLine={form.setToLine}
headers={form.headers}
parametersList={form.parametersList}
onParametersChange={form.onParameterChange}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ export function useRunBatchForm({

setSelectedDataset(ds)
setParameters(buildEmptyParameters(parametersList))
setFromLine(0)
setFromLine(1)
setToLine(ds.fileMetadata.rowCount)
setHeaders([
{ value: '-1', label: '-- Leave this parameter empty' },
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { ReactNode } from 'react'
import { EvaluationsRepository } from '@latitude-data/core/repositories'
import { computeEvaluationResultsWithMetadata } from '@latitude-data/core/services/evaluationResults/computeEvaluationResultsWithMetadata'
import { TableWithHeader, Text } from '@latitude-data/web-ui'
import { findCommitCached } from '$/app/(private)/_data-access'
import BreadcrumpLink from '$/components/BreadcrumpLink'
import { Breadcrump } from '$/components/layouts/AppLayout/Header'
import { getCurrentUser } from '$/services/auth/getCurrentUser'
Expand Down Expand Up @@ -30,10 +31,15 @@ export default async function ConnectedEvaluationLayout({
.find(params.evaluationId)
.then((r) => r.unwrap())

const commit = await findCommitCached({
projectId: Number(params.projectId),
uuid: params.commitUuid,
})
const evaluationResults = await computeEvaluationResultsWithMetadata({
workspaceId: evaluation.workspaceId,
evaluation,
documentUuid: params.documentUuid,
draft: commit,
}).then((r) => r.unwrap())

return (
Expand Down
4 changes: 2 additions & 2 deletions apps/web/src/app/layout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,8 @@ export default function RootLayout({
<NextTopLoader showSpinner={false} />
<ThemeProvider
attribute='class'
defaultTheme='system'
enableSystem
defaultTheme='light'
enableSystem={false}
disableTransitionOnChange
>
<TooltipProvider>{children}</TooltipProvider>
Expand Down
9 changes: 7 additions & 2 deletions apps/web/src/components/EditorHeader/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -53,12 +53,14 @@ export default function EditorHeader({
metadata,
onChangePrompt,
rightActions,
disabledMetadataSelectors = false,
}: {
title: string
metadata: ConversationMetadata | undefined
prompt: string
onChangePrompt: (prompt: string) => void
rightActions?: ReactNode
disabledMetadataSelectors?: boolean
}) {
const promptMetadata = useMemo<PromptMeta>(() => {
const config = metadata?.config
Expand Down Expand Up @@ -204,13 +206,16 @@ export default function EditorHeader({
<Select
name='provider'
label='Provider'
placeholder='Select a provider'
placeholder='Using default provider'
options={providerOptions}
value={selectedProvider}
disabled={
disabledMetadataSelectors || isLoading || !providerOptions.length
}
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

does the placeholder show when disabled? we should tell the user they are using the default provider if none selected

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If non selected it says "default provider" yes

onChange={onSelectProvider}
/>
<Select
disabled={!selectedProvider}
disabled={disabledMetadataSelectors || !selectedProvider}
name='model'
label='Model'
placeholder='Select a model'
Expand Down
5 changes: 3 additions & 2 deletions apps/web/src/components/layouts/AppLayout/Header/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ import {
SessionUser,
Text,
} from '@latitude-data/web-ui'
import { ThemeButton } from '$/components/ThemeButton'
// TODO: Review dark mode before enabling
// import { ThemeButton } from '$/components/ThemeButton'
import Link from 'next/link'
import { usePathname } from 'next/navigation'
import { Fragment } from 'react/jsx-runtime'
Expand Down Expand Up @@ -123,7 +124,7 @@ export default function AppHeader({
))}
</nav>
<AvatarDropdown currentUser={currentUser} />
<ThemeButton />
{/* <ThemeButton /> Not good enough for Cesar */}
</div>
</div>
{sectionLinks.length > 0 ? (
Expand Down
12 changes: 8 additions & 4 deletions packages/core/src/lib/readCsv.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import { isNumber } from 'lodash-es'

import { CsvError, parse, type Options as CsvOptions } from 'csv-parse/sync'

import { Result } from './Result'
Expand Down Expand Up @@ -40,12 +42,14 @@ export async function syncReadCsv(
info: true,
}

if (toLine) {
opts = { ...opts, toLine }
if (isNumber(fromLine)) {
// from: https://csv.js.org/parse/options/from/
opts = { ...opts, from: fromLine }
}

if (fromLine) {
opts = { ...opts, fromLine }
if (toLine) {
// to: https://csv.js.org/parse/options/to/
opts = { ...opts, to: toLine }
}

const records = parse(data, opts) as ParseResult[]
Expand Down
2 changes: 1 addition & 1 deletion packages/core/src/services/datasets/preview.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ export async function previewDataset({
dataset,
disk = diskFactory(),
prependIndex,
fromLine = 0,
fromLine = 1,
toLine = 100,
}: {
dataset: Dataset
Expand Down
2 changes: 1 addition & 1 deletion packages/env/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ if (environment !== 'production') {
WEBSOCKET_REFRESH_SECRET_TOKEN_KEY: 'refresh-refresh-token-key',
WORKERS_WEBSOCKET_SECRET_TOKEN: 'workers-secret-token',
DRIVE_DISK: 'local',
DEFAULT_PROVIDER_ID: '1',
DEFAULT_PROVIDER_ID: '-1', // Crazy number to avoid conflicts
DEFAULT_PROVIDER_MODEL: 'gpt-4o',
FILE_PUBLIC_PATH,
FILES_STORAGE_PATH,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,12 +28,13 @@ vi.mock('@latitude-data/core/data-access', () => ({
findWorkspaceFromDocument: vi.fn(),
}))

const commitReo = vi.hoisted(() => ({
getCommitByUuid: vi.fn().mockResolvedValue({
unwrap: () => ({ id: 'commit-1' }),
}),
}))
vi.mock('@latitude-data/core/repositories', () => ({
CommitsRepository: vi.fn().mockImplementation(() => ({
find: vi.fn().mockResolvedValue({
unwrap: () => ({ id: 'commit-1' }),
}),
})),
CommitsRepository: vi.fn().mockImplementation(() => commitReo),
}))

vi.mock('@latitude-data/core/services/datasets/preview', () => ({
Expand Down Expand Up @@ -70,6 +71,8 @@ describe('runBatchEvaluationJob', () => {
evaluation: { id: 1 },
dataset: { fileMetadata: { rowCount: 3 } },
document: { documentUuid: 'fake-document-uuid', commitId: 'commit-1' },
commitUuid: 'commit-uuid-1',
projectId: 1,
parametersMap: { param1: 0, param2: 1 },
},
attemptsMade: 0,
Expand Down Expand Up @@ -118,6 +121,15 @@ describe('runBatchEvaluationJob', () => {
})
})

it('find commit by uuid and project Id', async () => {
await runBatchEvaluationJob(mockJob)

expect(commitReo.getCommitByUuid).toHaveBeenCalledWith({
projectId: 1,
uuid: 'commit-uuid-1',
})
})

it('should process all rows and enqueue jobs', async () => {
await runBatchEvaluationJob(mockJob)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ type RunBatchEvaluationJobParams = {
evaluation: EvaluationDto
dataset: Dataset
document: DocumentVersion
commitUuid: string
projectId: number
fromLine?: number
toLine?: number
parametersMap?: Record<string, number>
Expand All @@ -33,7 +35,9 @@ export const runBatchEvaluationJob = async (
evaluation,
dataset,
document,
fromLine = 0,
projectId,
commitUuid,
fromLine,
toLine,
parametersMap,
batchId = randomUUID(),
Expand All @@ -43,7 +47,7 @@ export const runBatchEvaluationJob = async (
if (!workspace) throw new NotFoundError('Workspace not found')

const commit = await new CommitsRepository(workspace.id)
.find(document.commitId)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't get this, how can document.commitid be different than commitUuid? the documentversion always poins to its commit no?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before it was the commitId the document was created (live). Now is the commit that the user is seeing. live or a draft commit. So, the logs are attached to that commit. Which is what we want

.getCommitByUuid({ projectId, uuid: commitUuid })
.then((r) => r.unwrap())
const fileMetadata = dataset.fileMetadata
// TODO: use streaming instead of this service in order to avoid loading the
Expand Down
Loading