Skip to content

Commit

Permalink
feat: feedback cascading, filters v2, Users page search & pagination (#…
Browse files Browse the repository at this point in the history
…153)

* Feat: feedback & user filters improvements

* fix labels

* feat: search & pagination on users page

* remove column

* fix slow query

* cache dependencies

* test cache

* test cache

---------

Co-authored-by: Hugues Chocart <[email protected]>
  • Loading branch information
vincelwt and hughcrt authored Mar 29, 2024
1 parent 2cd3860 commit 691ab0a
Show file tree
Hide file tree
Showing 33 changed files with 550 additions and 207 deletions.
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

0 comments on commit 691ab0a

Please sign in to comment.