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

Pipipi #3087

Merged
merged 14 commits into from
Dec 20, 2024
Merged

Pipipi #3087

Show file tree
Hide file tree
Changes from 13 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
45 changes: 33 additions & 12 deletions bifrost/app/pi/first_page/page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,30 +6,51 @@ import { useRouter } from "next/navigation";
import { Suspense } from "react";
import { useHeliconeLogin } from "./../useHeliconeLogin";
import { useTestAPIKey } from "./useTestApiKey";
import { useQuery } from "@tanstack/react-query";

const jetbrainsMono = JetBrains_Mono({ subsets: ["latin"] });

const FirstPageContent = () => {
const jawn = useJawnClient();
const { apiKey, sessionUUID } = useHeliconeLogin();
const { data, isLoading } = useTestAPIKey(apiKey.data ?? "");
const jawn = useJawnClient(apiKey.data ?? "");
Comment on lines 15 to +16
Copy link

Choose a reason for hiding this comment

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

logic: passing empty string as fallback for apiKey.data could trigger unnecessary API calls - consider adding enabled: !!apiKey.data to useTestAPIKey

const router = useRouter();

Copy link

Choose a reason for hiding this comment

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

logic: all three useQuery hooks should include enabled: !!apiKey.data to prevent unnecessary API calls when apiKey is not available

if ((!data && !isLoading) || !apiKey.data) {
router.push("/pi/total-requests");
const orgName = useQuery({
queryKey: ["org-name", apiKey.data],
queryFn: () => jawn.POST("/v1/pi/org-name/query"),
});

const totalCosts = useQuery({
queryKey: ["total-costs", apiKey.data],
queryFn: () => jawn.POST("/v1/pi/total-costs"),
});

const costsOverTime = useQuery({
queryKey: ["costs-over-time", apiKey.data],
queryFn: () =>
jawn.POST("/v1/pi/costs-over-time/query", {
body: {
userFilter: "all",
dbIncrement: "day",
timeZoneDifference: new Date().getTimezoneOffset(),
timeFilter: {
start: new Date(
Date.now() - 30 * 24 * 60 * 60 * 1000
).toISOString(),
end: new Date().toISOString(),
},
},
}),
});

if (!data && !isLoading && !apiKey.data && !apiKey.isLoading) {
router.push("/pi/setup?invalid_api_key=true");
Comment on lines +46 to 49
Copy link

Choose a reason for hiding this comment

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

style: showing Loading... while redirecting may cause UI flicker - consider removing the loading message since the redirect happens immediately

return <div>Loading...</div>;
}
Copy link

Choose a reason for hiding this comment

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

logic: returning an empty div after fetching data is likely unintentional - component should render fetched data or loading states


return (
<div className="w-full flex flex-col justify-center items-center h-[100vh]">
<h1
className={`text-3xl font-extrabold truncate max-w-[80vw] ${jetbrainsMono.className} py-2`}
>
LOGGED IN! WOOOHOO
</h1>
{data && JSON.stringify(data).slice(0, 100)}
</div>
);
return <div></div>;
};

const PiPage = () => {
Expand Down
10 changes: 5 additions & 5 deletions bifrost/app/pi/page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,10 @@ const jetbrainsMono = JetBrains_Mono({ subsets: ["latin"] });

const PiPageContent = () => {
return (
<div className="w-full flex flex-col justify-center items-center h-[100vh]">
<h1
className={`text-3xl font-extrabold truncate max-w-[80vw] ${jetbrainsMono.className} py-2`}
>
<div
className={`w-full flex flex-col justify-center items-center h-[100vh] ${jetbrainsMono.className}`}
>
<h1 className={`text-3xl font-extrabold truncate max-w-[80vw] py-2`}>
Copy link

Choose a reason for hiding this comment

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

syntax: extra space before py-2 in className string

Welcome to Helicone!
</h1>

Expand All @@ -31,7 +31,7 @@ const PiPageContent = () => {

<Link
href="/pi/setup"
className=" bg-blue-500 p-5 bg-opacity-60 absolute top-1/2 left-1/2 transform translate-x-[30px] -translate-y-[110px] text-white font-bold"
className="p-5 border-2 absolute top-1/2 left-1/2 transform translate-x-[16px] -translate-y-[90px] text-black font-bold h-[36.17px] px-[12.17px] py-[6.08px] origin-top-left -rotate-1 bg-white border-l-2 border-r-4 border-t-2 border-b-4 border-black flex-col justify-start items-start gap-[10.14px] inline-flex"
Copy link

Choose a reason for hiding this comment

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

syntax: extra space after border-2 in className string

Copy link

Choose a reason for hiding this comment

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

logic: height and padding values conflict - h-[36.17px] with p-5 will make element taller than specified height

>
Start
</Link>
Expand Down
2 changes: 1 addition & 1 deletion bifrost/app/pi/setup/page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ const PiPageContent = () => {
);
const { data, isLoading } = useTestAPIKey(apiKey.data ?? "");

if (apiKey.data && data && !isLoading) {
if (apiKey.data && data && !isLoading && !apiKey.isLoading) {
router.push("/pi/first_page");
return <div>Loading...</div>;
}
Copy link

Choose a reason for hiding this comment

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

style: this loading state is confusing - showing a loading message after already initiating navigation could cause a flash of content. Consider returning null or using a more descriptive loading state

Expand Down
110 changes: 110 additions & 0 deletions bifrost/app/pi/total-cost/page.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,110 @@
"use client";

import { useRouter } from "next/navigation";
import { useHeliconeLogin } from "../useHeliconeLogin";
import Link from "next/link";

import { JetBrains_Mono } from "next/font/google";
import { useTestAPIKey } from "../first_page/useTestApiKey";
import { useJawnClient } from "@/lib/clients/jawnHook";
import { useQuery } from "@tanstack/react-query";
import { Bar, BarChart, ResponsiveContainer, XAxis, YAxis } from "recharts";
const jetbrainsMono = JetBrains_Mono({ subsets: ["latin"] });

const PiGraphLayout = ({ children }: { children: React.ReactNode }) => {
const { apiKey } = useHeliconeLogin();
const { data, isLoading } = useTestAPIKey(apiKey.data ?? "");
const router = useRouter();

const jawn = useJawnClient(apiKey.data ?? "");

const totalRequests = useQuery({
queryKey: ["total-requests", apiKey.data],
queryFn: () => jawn.POST("/v1/pi/total_requests"),
});
Comment on lines +28 to +31
Copy link

Choose a reason for hiding this comment

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

logic: totalRequests is fetched but never used in the component


const costsOverTime = useQuery({
Copy link

Choose a reason for hiding this comment

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

logic: no error handling or loading state for costsOverTime query

queryKey: ["costs-over-time", apiKey.data],
queryFn: () =>
jawn.POST("/v1/pi/costs-over-time/query", {
body: {
userFilter: "all",
dbIncrement: "day",
timeZoneDifference: new Date().getTimezoneOffset(),
timeFilter: {
start: new Date(
Date.now() - 30 * 24 * 60 * 60 * 1000
).toISOString(),
end: new Date().toISOString(),
},
},
}),
});
const costsOverTimeData = costsOverTime.data?.data?.data;

if (!data && !isLoading && !apiKey.data && !apiKey.isLoading) {
router.push("/pi/setup?invalid_api_key=true");
return null;
}

return (
<div
className={`w-full flex flex-col justify-center items-center h-[100vh] p-5 ${jetbrainsMono.className}`}
>
<div className="w-full h-[400px] p-4 bg-background">
<h2 className="text-2xl font-bold mb-4">Costs</h2>
<div className="text-3xl font-bold">
$
{costsOverTimeData
?.reduce((sum, item) => sum + item.cost, 0)
?.toFixed(2) ?? "0.00"}
</div>
<ResponsiveContainer width="100%" height={300}>
<BarChart data={costsOverTimeData ?? []}>
<XAxis
dataKey="created_at_trunc"
tickFormatter={(value) =>
new Date(value).toLocaleDateString(undefined, {
month: "numeric",
day: "numeric",
})
}
fontSize={12}
interval={4} // Show every 4th tick
/>
<YAxis fontSize={12} />
<Bar
dataKey="cost"
fill="#888888"
radius={[0, 0, 0, 0]}
stroke="#000000" // Add black outline
strokeWidth={1} // Set outline thickness
/>
</BarChart>
</ResponsiveContainer>
</div>
<div className="h-full w-full">{children}</div>
<div className="flex justify-between items-center w-full">
<Link href="/pi/total-requests">
{/* eslint-disable-next-line @next/next/no-img-element */}
<img
src="/static/pi/arrow-left.png"
alt="arrow-left"
className="w-16 h-16"
/>
</Link>

<Link href="/pi/total-requests">
{/* eslint-disable-next-line @next/next/no-img-element */}
<img
src="/static/pi/arrow-right.png"
alt="arrow-right"
className="w-16 h-16"
/>
</Link>
</div>
</div>
);
};

export default PiGraphLayout;
175 changes: 175 additions & 0 deletions bifrost/app/pi/total-requests/page.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,175 @@
"use client";

import { useRouter } from "next/navigation";
import { useHeliconeLogin } from "../useHeliconeLogin";

import { JetBrains_Mono } from "next/font/google";
import { useTestAPIKey } from "../first_page/useTestApiKey";
import { useJawnClient } from "@/lib/clients/jawnHook";
import { useQuery } from "@tanstack/react-query";
const jetbrainsMono = JetBrains_Mono({ subsets: ["latin"] });

const formatNumber = (
number: number
): [
number | null,
number | null,
"." | "," | number | null,
number | null | ".",
number | null,
number | null | "B" | "M"
] => {
Comment on lines +13 to +22
Copy link

Choose a reason for hiding this comment

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

style: formatNumber function handles number formatting but may have edge cases with negative numbers or decimals

if (number < 10) {
return [null, null, null, null, null, number];
} else if (number < 100) {
return [
null,
null,
null,
null,
Number(number.toString().split("")[0]),
Number(number.toString().split("")[1]),
];
} else if (number < 1000) {
return [
null,
null,
null,
Number(number.toString().split("")[0]),
Number(number.toString().split("")[1]),
Number(number.toString().split("")[2]),
];
} else if (number < 10000) {
return [
null,
Number(number.toString().split("")[0]),
",",
Number(number.toString().split("")[1]),
Number(number.toString().split("")[2]),
Number(number.toString().split("")[3]),
];
} else if (number < 100_000) {
return [
Number(number.toString().split("")[0]),
Number(number.toString().split("")[1]),
",",
Number(number.toString().split("")[2]),
Number(number.toString().split("")[3]),
Number(number.toString().split("")[4]),
];
} else if (number < 1_000_000) {
// 2,345,678 = _,2,.,3,4,M
return [
null,
Number(number.toString().split("")[0]),
".",
Number(number.toString().split("")[1]),
Number(number.toString().split("")[2]),
"M",
];
} else if (number < 10_000_000) {
// 23,456,789 = 2,3,.,4,5,M
return [
Number(number.toString().split("")[0]),
Number(number.toString().split("")[1]),
".",
Number(number.toString().split("")[2]),
Number(number.toString().split("")[3]),
"M",
];
} else if (number < 100_000_000) {
// 234,567,890 = 2,3,4,.,5,M
return [
Number(number.toString().split("")[0]),
Number(number.toString().split("")[1]),
Number(number.toString().split("")[2]),
".",
Number(number.toString().split("")[3]),
"M",
];
} else if (number < 1_000_000_000) {
// 245,678,901 = _,_,2,4,5,M
return [
null,
null,
Number(number.toString().split("")[0]),
Number(number.toString().split("")[1]),
Number(number.toString().split("")[2]),
"M",
];
} else if (number < 10_000_000_000) {
// 1,234,567,890 = _,1,.,2,3,B
return [
null,
Number(number.toString().split("")[0]),
".",
Number(number.toString().split("")[1]),
Number(number.toString().split("")[2]),
"B",
];
} else {
return [null, null, null, null, null, null];
}
};

const TotalRequestsPage = () => {
const { apiKey } = useHeliconeLogin();
const { data, isLoading } = useTestAPIKey(apiKey.data ?? "");
const router = useRouter();

const jawn = useJawnClient(apiKey.data ?? "");

const totalRequests = useQuery({
queryKey: ["total-requests", apiKey.data],
queryFn: () => jawn.POST("/v1/pi/total_requests"),
});

if (!data && !isLoading && !apiKey.data && !apiKey.isLoading) {
router.push("/pi/setup?invalid_api_key=true");
return null;
}

return (
<div
className={`w-full flex flex-col justify-center items-center h-[100vh] p-5 gap-4 ${jetbrainsMono.className}`}
>
<div className="h-full w-full bg-[#E5E5E5] border-2 border-black flex flex-col items-center justify-center gap-7">
<div className="bg-white p-2.5 border-2 border-black border-r-4 border-b-4 font-medium text-[21px]">
Total Requests
</div>{" "}
<div className="flex gap-2">
{formatNumber(totalRequests.data?.data?.data ?? 0).map(
Copy link

Choose a reason for hiding this comment

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

logic: no error handling for failed API requests - should handle totalRequests.error and totalRequests.isLoading states

(num, index) => (
<div
key={index}
className="h-[120px] w-[82px] bg-white border-2 border-black border-r-4 border-b-4 text-[76px] font-extrabold flex items-center justify-center"
>
{num}
</div>
)
)}
</div>
<div className="bg-white p-2.5 border-2 border-black border-r-4 border-b-4 font-medium text-[14px]">
Last 30 days
</div>
{/* {JSON.stringify(formatNumber(totalRequests.data?.data?.data ?? 0))} */}
</div>
<div className="flex w-full justify-between items-center">
<img
src="/static/pi/arrow-left.png"
alt="arrow-left"
className="w-16 h-16"
/>
<div className="flex flex-col items-center">
<img
src="/static/pi/arrow-right.png"
alt="arrow-left"
Copy link

Choose a reason for hiding this comment

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

syntax: incorrect alt text 'arrow-left' for right arrow image

className="w-16 h-16"
/>
</div>
Copy link

Choose a reason for hiding this comment

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

logic: navigation arrows are rendered but not wrapped in Link components or onClick handlers

</div>
</div>
);
};

export default TotalRequestsPage;
1 change: 1 addition & 0 deletions bifrost/app/pi/useHeliconeLogin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ export const useHeliconeLogin = (invalid_api_key?: boolean) => {
return apiKey.data?.data?.apiKey;
},
refetchInterval: 10_000, // 1 second
staleTime: 0,
Comment on lines 80 to +81
Copy link

Choose a reason for hiding this comment

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

style: setting staleTime to 0 with a 10 second refetchInterval will cause frequent network requests - consider if caching for a few seconds would be acceptable to reduce load

});

return {
Expand Down
Loading
Loading