Skip to content

Commit d64ad7e

Browse files
hcharlie1201hcharlie1201
committed
chores: addressed comments
1 parent 752a64f commit d64ad7e

File tree

8 files changed

+105
-63
lines changed

8 files changed

+105
-63
lines changed

‎clickhouse/migrations/schema_46_tags.sql‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,11 @@
11
CREATE TABLE IF NOT EXISTS default.tags (
2+
organization_id UUID,
23
entity_type String,
34
entity_id String,
45
tag String,
56
created_at DateTime DEFAULT now(),
6-
PRIMARY KEY (entity_type, entity_id, tag)
7+
PRIMARY KEY (organization_id, entity_type, entity_id, tag)
78
) ENGINE = ReplacingMergeTree()
8-
ORDER BY (entity_type, entity_id, tag);
9+
ORDER BY (organization_id, entity_type, entity_id, tag);
910

1011
-- https://clickhouse.com/docs/guides/developer/deduplication

‎valhalla/jawn/src/lib/db/ClickhouseWrapper.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -244,6 +244,7 @@ export interface JawnHttpLogs {
244244
}
245245

246246
export interface Tags {
247+
organization_id: string;
247248
entity_type: string;
248249
entity_id: string;
249250
tag: string;

‎valhalla/jawn/src/lib/shared/filters/filters.ts‎

Lines changed: 20 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -380,8 +380,7 @@ export function buildFilterLeaf(
380380
return argPlaceHolder(argsAcc.length - 1, value);
381381
};
382382

383-
const filters: string[] = [];
384-
for (const _tableKey in filter) {
383+
const filters = Object.keys(filter).reduce<string[]>((acc, _tableKey) => {
385384
const tableKey = _tableKey as keyof typeof filter;
386385
const table = filter[tableKey];
387386
// table is {session_tag: { equals: "test" }} tableKey is sessions_request_response_rmt
@@ -394,7 +393,7 @@ export function buildFilterLeaf(
394393
} = mapper(table, placeValueSafely);
395394

396395
if (!column) {
397-
continue;
396+
return acc;
398397
}
399398

400399
const tagFilter =
@@ -411,30 +410,26 @@ export function buildFilterLeaf(
411410
AND entity_type = '${TagType.SESSION}'
412411
`;
413412

414-
filters.push(`properties['Helicone-Session-Id'] IN (${subQuery})`);
415-
continue;
413+
return [...acc, `properties['Helicone-Session-Id'] IN (${subQuery})`];
416414
}
417415

418-
switch (true) {
419-
case operatorKey === "not-equals" && value === "null":
420-
filters.push(`${column} is not null`);
421-
break;
422-
case operatorKey === "equals" && value === "null":
423-
filters.push(`${column} is null`);
424-
break;
425-
case operatorKey === "contains" || operatorKey === "not-contains":
426-
filters.push(`${column} ${sqlOperator} '%' || ${value}::text || '%'`);
427-
break;
428-
case operatorKey === "vector-contains":
429-
filters.push(
430-
`${column} ${sqlOperator} plainto_tsquery('helicone_search_config', ${value}::text)`
431-
);
432-
break;
433-
default:
434-
filters.push(`${column} ${sqlOperator} ${value}`);
435-
break;
436-
}
437-
}
416+
const filterClause = (() => {
417+
switch (true) {
418+
case operatorKey === "not-equals" && value === "null":
419+
return `${column} is not null`;
420+
case operatorKey === "equals" && value === "null":
421+
return `${column} is null`;
422+
case operatorKey === "contains" || operatorKey === "not-contains":
423+
return `${column} ${sqlOperator} '%' || ${value}::text || '%'`;
424+
case operatorKey === "vector-contains":
425+
return `${column} ${sqlOperator} plainto_tsquery('helicone_search_config', ${value}::text)`;
426+
default:
427+
return `${column} ${sqlOperator} ${value}`;
428+
}
429+
})();
430+
431+
return [...acc, filterClause];
432+
}, []);
438433

439434
return {
440435
filters,

‎valhalla/jawn/src/managers/SessionManager.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -405,14 +405,15 @@ export class SessionManager {
405405
const query = `
406406
SELECT tag
407407
FROM tags
408-
WHERE entity_id = {val_0: String} AND entity_type = {val_1: String}
408+
WHERE entity_id = {val_0: String} AND entity_type = {val_1: String} AND organization_id = {val_2: String}
409409
GROUP BY tag
410410
ORDER BY max(created_at) DESC
411411
`;
412412

413413
const result = await clickhouseDb.dbQuery<{ tag: string }>(query, [
414414
sessionId,
415415
TagType.SESSION,
416+
this.authParams.organizationId,
416417
]);
417418

418419
if (result.error) {
@@ -436,6 +437,7 @@ export class SessionManager {
436437
): Promise<Result<null, string>> {
437438
try {
438439
const valuesToInsert: Tags = {
440+
organization_id: this.authParams.organizationId,
439441
entity_type: TagType.SESSION,
440442
entity_id: sessionId,
441443
tag: tag,

‎web/components/templates/feedback/sessionTag.tsx‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { useOrg } from "@/components/layout/org/organizationContext";
12
import useNotification from "@/components/shared/notification/useNotification";
23
import { Button } from "@/components/ui/button";
34
import {
@@ -24,16 +25,15 @@ interface SessionTagProps {
2425
}
2526

2627
export const SessionTag = ({ id, type }: SessionTagProps) => {
28+
const currentOrgId = useOrg()?.currentOrg?.id;
2729
const { getTag, setTag } = useTagStore();
28-
const [formTag, setFormTag] = useState(getTag(id, type) || "");
30+
const [formTag, setFormTag] = useState(getTag(currentOrgId!, id, type) || "");
2931
const { setNotification } = useNotification();
3032
const [open, setOpen] = useState(false);
3133
useEffect(() => {
32-
fetchTag(id, type).then((res) => {
34+
fetchTag(currentOrgId!, id, type, setTag).then((res) => {
3335
if (res?.error) {
3436
setNotification("Error fetching tag", "error");
35-
} else if (res?.data?.data) {
36-
setTag(id, res.data.data, type);
3737
}
3838
});
3939
// eslint-disable-next-line react-hooks/exhaustive-deps
@@ -42,13 +42,15 @@ export const SessionTag = ({ id, type }: SessionTagProps) => {
4242
return (
4343
<Dialog open={open} onOpenChange={setOpen}>
4444
<DialogTrigger asChild>
45-
<Button variant="outline">{getTag(id, type) || "Add Tag"}</Button>
45+
<Button variant="outline">
46+
{getTag(currentOrgId!, id, type) || "Add Tag"}
47+
</Button>
4648
</DialogTrigger>
4749
<DialogContent className="sm:max-w-[425px]">
4850
<DialogHeader>
4951
<DialogTitle>Edit Tag</DialogTitle>
5052
<DialogDescription>
51-
Make changes to your tag here. Click save when youre done.
53+
Make changes to your tag here. Click save when you&apos;re done.
5254
</DialogDescription>
5355
</DialogHeader>
5456
<form
@@ -59,12 +61,11 @@ export const SessionTag = ({ id, type }: SessionTagProps) => {
5961
return;
6062
}
6163

62-
updateTag(id, formTag, type).then((res) => {
64+
updateTag(currentOrgId!, id, formTag, type, setTag).then((res) => {
6365
if (res?.error) {
6466
setNotification("Error updating tag", "error");
6567
} else {
6668
setOpen(false);
67-
setTag(id, formTag, type);
6869
setNotification("Tag updated", "success");
6970
}
7071
});

‎web/package.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@
8282
"framer-motion": "^11.11.11",
8383
"fuse.js": "^7.0.0",
8484
"generate-api-key": "^1.0.2",
85+
"immer": "^10.1.1",
8586
"js-cookie": "^3.0.5",
8687
"lucide-react": "^0.487.0",
8788
"marked": "^13.0.3",

‎web/store/features/sessions/tag.ts‎

Lines changed: 63 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -1,58 +1,94 @@
11
import { $JAWN_API } from "@/lib/clients/jawn";
22
import { TagType } from "../../../packages/common/sessions/tags";
33
import { create } from "zustand";
4+
import { produce } from "immer";
45

5-
interface TagStore {
6+
type TagStoreState = {
67
tagStore: {
7-
[key in TagType]: {
8-
[id: string]: string;
8+
[ordId: string]: {
9+
[key in TagType]: {
10+
[id: string]: string;
11+
};
912
};
1013
};
11-
getTag: (id: string, type: TagType) => string | undefined;
12-
setTag: (id: string, tag: string, type: TagType) => void;
13-
}
14+
};
15+
type TagStoreAction = {
16+
getTag: (orgId: string, session: string, type: TagType) => string | undefined;
17+
setTag: (
18+
orgId: string,
19+
sessionId: string,
20+
tag: string,
21+
type: TagType
22+
) => void;
23+
};
1424

15-
export const useTagStore = create<TagStore>((set, get) => ({
16-
tagStore: {
17-
[TagType.REQUEST]: {},
18-
[TagType.SESSION]: {},
19-
},
20-
getTag: (id: string, type: TagType) => get().tagStore[type][id],
21-
setTag: (id: string, tag: string, type: TagType) =>
22-
set((state) => ({
23-
tagStore: {
24-
...state.tagStore,
25-
[type]: {
26-
...state.tagStore[type],
27-
[id]: tag,
28-
},
29-
},
30-
})),
31-
}));
25+
export const useTagStore = create<TagStoreState & TagStoreAction>(
26+
(set, get) => ({
27+
tagStore: {},
28+
29+
getTag: (orgId: string, sessionId: string, type: TagType) =>
30+
get().tagStore[orgId]?.[type]?.[sessionId],
31+
32+
setTag: (orgId: string, sessionId: string, tag: string, type: TagType) =>
33+
set(
34+
produce((state) => {
35+
// Initialize with empty objects using nullish coalescing
36+
state.tagStore[orgId] = state.tagStore[orgId] ?? {};
37+
state.tagStore[orgId][type] = state.tagStore[orgId][type] ?? {};
38+
state.tagStore[orgId][type][sessionId] = tag;
39+
})
40+
),
41+
})
42+
);
3243

33-
export async function fetchTag(id: string, type: TagType) {
44+
export async function fetchTag(
45+
orgId: string,
46+
sessionId: string,
47+
type: TagType,
48+
setTag: TagStoreAction["setTag"]
49+
) {
3450
if (type === TagType.SESSION) {
3551
const response = await $JAWN_API.GET("/v1/session/{sessionId}/tag", {
3652
params: {
37-
path: { sessionId: id },
53+
path: { sessionId: sessionId },
3854
},
3955
});
4056

57+
if (!response.error && response.data?.data) {
58+
setTag(orgId, sessionId, response.data?.data, TagType.SESSION);
59+
}
60+
4161
return response;
4262
}
63+
64+
// TODO: Implement fetching tags for other types
65+
throw new Error(`Fetching tags for ${type} is not implemented`);
4366
}
4467

45-
export async function updateTag(id: string, tag: string, type: TagType) {
68+
export async function updateTag(
69+
orgId: string,
70+
sessionId: string,
71+
tag: string,
72+
type: TagType,
73+
setTag: TagStoreAction["setTag"]
74+
) {
4675
if (type === TagType.SESSION) {
4776
const response = await $JAWN_API.POST("/v1/session/{sessionId}/tag", {
4877
params: {
49-
path: { sessionId: id },
78+
path: { sessionId: sessionId },
5079
},
5180
body: {
5281
tag: tag,
5382
},
5483
});
5584

85+
if (!response.error) {
86+
setTag(orgId, sessionId, tag, TagType.SESSION);
87+
}
88+
5689
return response;
5790
}
91+
92+
// TODO: Implement updating tags for other types
93+
throw new Error(`Updating tags for ${type} is not implemented`);
5894
}

‎web/yarn.lock‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7919,6 +7919,11 @@ ignore@^5.2.0:
79197919
resolved "https://registry.npmjs.org/ignore/-/ignore-5.3.1.tgz"
79207920
integrity sha512-5Fytz/IraMjqpwfd34ke28PTVMjZjJG2MPn5t7OE4eUCUNf8BAa7b5WUS9/Qvr6mwOQS7Mk6vdsMno5he+T8Xw==
79217921

7922+
immer@^10.1.1:
7923+
version "10.1.1"
7924+
resolved "https://registry.yarnpkg.com/immer/-/immer-10.1.1.tgz#206f344ea372d8ea176891545ee53ccc062db7bc"
7925+
integrity sha512-s2MPrmjovJcoMaHtx6K11Ra7oD05NT97w1IC5zpMkT6Atjr7H8LjaDd81iIxUYpMKSRRNMJE703M1Fhr/TctHw==
7926+
79227927
import-fresh@^3.1.0, import-fresh@^3.2.1, import-fresh@^3.3.0:
79237928
version "3.3.0"
79247929
resolved "https://registry.npmjs.org/import-fresh/-/import-fresh-3.3.0.tgz"

0 commit comments

Comments
 (0)