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

Feat: feedback cascading, filters v2, Users page search & pagination #153

Merged
merged 9 commits into from
Mar 29, 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
20 changes: 19 additions & 1 deletion .github/workflows/test-build-deploy.yml
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,24 @@ jobs:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v4
- name: Cache node modules
id: cache-npm
uses: actions/cache@v3
env:
cache-name: cache-node-modules
with:
# npm cache files are stored in `~/.npm` on Linux/macOS
path: ~/.npm
key: ${{ runner.os }}-build-${{ env.cache-name }}-${{ hashFiles('**/package-lock.json') }}
restore-keys: |
${{ runner.os }}-build-${{ env.cache-name }}-
${{ runner.os }}-build-
${{ runner.os }}-

- if: ${{ steps.cache-npm.outputs.cache-hit != 'true' }}
name: List the state of node modules
continue-on-error: true
run: npm list
- name: Install dependencies
run: npm ci && npx playwright install --with-deps
- name: Start backend
Expand All @@ -18,7 +36,7 @@ jobs:
LUNARY_PUBLIC_KEY: 259d2d94-9446-478a-ae04-484de705b522
OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }}
timeout-minutes: 1
run: npm run start:backend & npx wait-on http://localhost:3333/v1/health
run: npm run start:backend & npx wait-on http://localhost:3333/v1/health

- name: Start frontend
env:
Expand Down
20 changes: 9 additions & 11 deletions packages/backend/src/api/v1/auth/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import Router from "koa-router"
import { z } from "zod"
import {
hashPassword,
requestPasswordReset,
sanitizeEmail,
signJwt,
verifyJwt,
Expand Down Expand Up @@ -235,8 +236,13 @@ auth.post("/login", async (ctx: Context) => {
select * from account where email = ${email}
`
if (!user) {
ctx.status = 403
ctx.body = { error: "Unauthorized", message: "Invalid email or password" }
ctx.body = ctx.throw(403, "Invalid email or password")
}

if (!user.passwordHash) {
await requestPasswordReset(email)

ctx.body = { message: "We sent you an email to reset your password" }
return
}

Expand Down Expand Up @@ -274,16 +280,8 @@ auth.post("/request-password-reset", async (ctx: Context) => {
}

const { email } = body.data
const [user] = await sql`select id from account where email = ${email}`

const ONE_HOUR = 60 * 60
const token = await signJwt({ email }, ONE_HOUR)

await sql`update account set recovery_token = ${token} where id = ${user.id}`

const link = `${process.env.APP_URL}/reset-password?token=${token}`

await sendEmail(RESET_PASSWORD(email, link))
await requestPasswordReset(email)

ctx.body = { ok: true }
} catch (error) {
Expand Down
15 changes: 15 additions & 0 deletions packages/backend/src/api/v1/auth/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ import * as argon2 from "argon2"

import bcrypt from "bcrypt"
import { validateUUID } from "@/src/utils/misc"
import { sendEmail } from "@/src/utils/sendEmail"
import { RESET_PASSWORD } from "@/src/utils/emails"

export function sanitizeEmail(email: string) {
return email.toLowerCase().trim()
Expand Down Expand Up @@ -132,3 +134,16 @@ export async function authMiddleware(ctx: Context, next: Next) {

await next()
}

export async function requestPasswordReset(email: string) {
const [user] = await sql`select id from account where email = ${email}`

const ONE_HOUR = 60 * 60
const token = await signJwt({ email }, ONE_HOUR)

await sql`update account set recovery_token = ${token} where id = ${user.id}`

const link = `${process.env.APP_URL}/reset-password?token=${token}`

await sendEmail(RESET_PASSWORD(email, link))
}
1 change: 0 additions & 1 deletion packages/backend/src/api/v1/evaluations/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,6 @@ evaluations.post(
queue.on("active", () => {
const percentDone = ((count - queue.size) / count) * 100
console.log(`Active: ${queue.size} of ${count} (${percentDone}%)`)
console.log()
stream.write(JSON.stringify({ percentDone }) + "\n")
})

Expand Down
15 changes: 9 additions & 6 deletions packages/backend/src/api/v1/external-users.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,11 +10,12 @@ const users = new Router({
users.get("/", checkAccess("users", "list"), async (ctx: Context) => {
const { projectId } = ctx.state

// const { limit, page } = ctx.query
const { limit = "100", page = "0", search } = ctx.query

// if(!limit || !page) {
// return ctx.throw(400, "limit and page are required")
// }
let searchQuery = sql``
if (search) {
searchQuery = sql`and (lower(external_id) ilike lower(${`%${search}%`}) or lower(props->>'email') ilike lower(${`%${search}%`}) or lower(props->>'name') ilike lower(${`%${search}%`}))`
}

// TODO: pagination
const users = await sql`
Expand All @@ -30,9 +31,11 @@ users.get("/", checkAccess("users", "list"), async (ctx: Context) => {
external_user
where
project_id = ${projectId}
${searchQuery}
order by
cost desc
`
cost desc
limit ${Number(limit)}
offset ${Number(page) * Number(limit)}`

ctx.body = users
})
Expand Down
54 changes: 5 additions & 49 deletions packages/backend/src/api/v1/filters.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import sql from "@/src/utils/db"
import Router from "koa-router"
import { Context } from "koa"
// import { checkAccess } from "@/src/utils/authorization"

const filters = new Router({
prefix: "/filters",
Expand All @@ -17,8 +16,6 @@ filters.get("/models", async (ctx: Context) => {
model_name_cache
where
project_id = ${projectId}
order by
project_id
`

ctx.body = rows.map((row) => row.name)
Expand All @@ -34,58 +31,24 @@ filters.get("/tags", async (ctx: Context) => {
tag_cache
where
project_id = ${projectId}
order by
project_id
`

ctx.body = rows.map((row) => row.tag)
})

filters.get("/feedback", async (ctx: Context) => {
const { projectId } = ctx.state
const { type } = ctx.query

const rows = await sql`
select
jsonb_build_object ('thumbs',
feedback::json ->> 'thumbs')
from
run
where
feedback::json ->> 'thumbs' is not null
and project_id = ${projectId}
union
select
jsonb_build_object ('emoji',
feedback::json ->> 'emoji')
from
run
where
feedback::json ->> 'emoji' is not null
and project_id = ${projectId}
union
select
jsonb_build_object ('rating',
CAST(feedback::json ->> 'rating' as INT))
from
run
where
feedback::json ->> 'rating' is not null
and project_id = ${projectId}
union
select
jsonb_build_object ('retried',
CAST(feedback::json ->> 'retried' as boolean))
feedback
from
run
feedback_cache
where
feedback::json ->> 'retried' is not null
and project_id = ${projectId}
project_id = ${projectId}
`

const feedbacks = rows.map((row) => row.jsonbBuildObject)

ctx.body = feedbacks
ctx.body = rows.map((row) => row.feedback) // stringify so it works with selected
})

// get all unique keys in metadata table
Expand All @@ -101,8 +64,6 @@ filters.get("/metadata", async (ctx: Context) => {
metadata_cache
where
project_id = ${projectId}
order by
project_id
`

ctx.body = rows.map((row) => row.key)
Expand All @@ -114,14 +75,11 @@ filters.get("/users", async (ctx) => {

const rows = await sql`
select
external_id as label,
id as value
*
from
external_user
where
project_id = ${projectId}
order by
project_id
`

ctx.body = rows
Expand All @@ -138,8 +96,6 @@ filters.get("/radars", async (ctx) => {
radar
where
project_id = ${projectId}
order by
project_id
`

ctx.body = rows
Expand Down
9 changes: 7 additions & 2 deletions packages/backend/src/api/v1/runs/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ const formatRun = (run: any) => ({
id: run.id,
isPublic: run.isPublic,
feedback: run.feedback,
parentFeedback: run.parentFeedback,

type: run.type,
name: run.name,
createdAt: run.createdAt,
Expand Down Expand Up @@ -120,10 +122,12 @@ runs.get("/", async (ctx: Context) => {
eu.external_id as user_external_id,
eu.created_at as user_created_at,
eu.last_seen as user_last_seen,
eu.props as user_props
eu.props as user_props,
rpfc.feedback as parent_feedback
from
run r
left join external_user eu on r.external_user_id = eu.id
left join run_parent_feedback_cache rpfc ON r.id = rpfc.id
where
r.project_id = ${projectId}
${parentRunCheck}
Expand Down Expand Up @@ -286,7 +290,8 @@ runs.get("/:id/related", checkAccess("logs", "read"), async (ctx) => {
ctx.body = related
})

runs.get("/:id/feedback", checkAccess("logs", "read"), async (ctx) => {
// public route
runs.get("/:id/feedback", async (ctx) => {
const { projectId } = ctx.state
const { id } = ctx.params

Expand Down
2 changes: 1 addition & 1 deletion packages/backend/src/api/v1/runs/ingest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ const registerRunEvent = async (
SET feedback = ${sql.json({
...((feedbackData?.feedback || {}) as any),
...feedback,
...extra,
...extra, // legacy
})}
WHERE id = ${runId}
`
Expand Down
22 changes: 22 additions & 0 deletions packages/backend/src/checks/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import aiSimilarity from "./ai/similarity"
// import aiNER from "./ai/ner"
// import aiToxicity from "./ai/toxic"
import rouge from "rouge"
import { or } from "../utils/checks"

function getTextsTypes(field: "any" | "input" | "output", run: any) {
let textsToCheck = []
Expand Down Expand Up @@ -111,6 +112,27 @@ export const CHECK_RUNNERS: CheckRunner[] = [
id: "users",
sql: ({ users }) => sql`external_user_id = ANY (${users})`,
},
{
id: "feedback",
sql: ({ types }) => {
// If one of the type is {"comment": ""}, we just need to check if there is a 'comment' key
// otherwise, we need to check for the key:value pair

return or(
types.map((type: string) => {
const parsedType = JSON.parse(type)
const key = Object.keys(parsedType)[0]
const value = parsedType[key]
if (key === "comment") {
// comment is a special case because there can be infinite values
return sql`r.feedback->${key} IS NOT NULL OR rpfc.feedback->${key} IS NOT NULL`
} else {
return sql`r.feedback->>${key} = ${value} OR rpfc.feedback->>${key} = ${value}`
}
}),
)
},
},
{
id: "regex",
evaluator: async (run, params) => {
Expand Down
4 changes: 2 additions & 2 deletions packages/backend/src/utils/checks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ import { CheckLogic, LogicElement } from "shared"
import sql from "./db"
import CHECK_RUNNERS from "../checks"

const and = (arr: any = []) =>
export const and = (arr: any = []) =>
arr.reduce((acc: any, x: any) => sql`${acc} AND ${x}`)
const or = (arr: any = []) =>
export const or = (arr: any = []) =>
arr.reduce((acc: any, x: any) => sql`(${acc} OR ${x})`)

// TODO: unit tests
Expand Down
1 change: 1 addition & 0 deletions packages/backend/src/utils/cron.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ export function setupCronJobs() {
"tag_cache",
"metadata_cache",
"feedback_cache",
"run_parent_feedback_cache",
]

try {
Expand Down
1 change: 1 addition & 0 deletions packages/backend/src/utils/emails.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ export function RESET_PASSWORD(email: string, confirmLink: string) {
text: `Hi,

Please click on the link below to reset your password:

${confirmLink}

You can reply to this email if you have any question.
Expand Down
2 changes: 0 additions & 2 deletions packages/backend/src/utils/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,9 +13,7 @@ export async function errorMiddleware(ctx: Context, next: Next) {
console.error(error)
sendErrorToSentry(error, ctx)

console.log(error)
if (error instanceof z.ZodError) {
console.log("HERE")
ctx.status = 422
ctx.body = {
error: "Error",
Expand Down
Loading
Loading