Helenlei/costing admin page #439
Conversation
|
[diff-counting] Significant lines: 261. |
There was a problem hiding this comment.
Overall, the implementation of the costing page was really well done, and used a clean UI that I thought was perfect for getting all of the behind-the-scenes information from the admin page -- I'll be drawing from that for the future. Only some small changes I left throughout the code to keep the codebase clean, but this will be useful for us moving forward!
| getCostingData(value, setTotalTokens); | ||
| }} | ||
| > | ||
| <option value="100000">x</option> |
There was a problem hiding this comment.
Maybe instead of setting the initial or "empty" state as an arbitrarily high limit for minimum reviews, it could just default to the lowest minimum number of reviews (3).
| @@ -6,6 +6,7 @@ import { Home } from './modules/Home' | |||
| import { Course } from './modules/Course' | |||
| import { Profile } from './modules/Profile' | |||
| import { Admin } from './modules/Admin' | |||
| import { AdminCosting } from './modules/AdminCosting' | |||
There was a problem hiding this comment.
One thing I'm not quite sure about, but could be a more clean way to separate the admin page and its functionality from the rest of the app - maybe Admin could contain a router of its own so that new pages to Admin are not all added to the overall app.
| function getCostingData(min: number, setTotalTokens: React.Dispatch<React.SetStateAction<number>>) { | ||
| console.log('loading costing data...'); | ||
|
|
||
| axios.post('/ai/costing', { min }) |
There was a problem hiding this comment.
It may be more clear what this route is related to if it was changed to something like /admin/aicosting
| if (res.data.result) { | ||
| setIsAdmin(true) | ||
| } else { | ||
| setIsAdmin(false) | ||
| } |
There was a problem hiding this comment.
I think you could condense this into setIsAdmin(res.data.result) for easier readability.
| <p>$1.50/1M tokens output</p> | ||
| </div> | ||
|
|
||
| <div className="other-numbers"> |
There was a problem hiding this comment.
Instead of "other-numbers", this class name could be "review-stats" or "review-numbers" to better represent the information rendered here.
|
|
||
| function adminLogin() { | ||
| if (loading) { | ||
| return <div>Loading...</div> |
There was a problem hiding this comment.
Once you pull Andrew's PR for the loading screen, replace "Loading..." with the loading spinner component using return <Loading />
| try { | ||
| const costing = await minReviewsCosting(min); | ||
| if (minReviewsCosting === null) { | ||
| return res.status(400).json({ | ||
| error: `No reviews found`, | ||
| }); | ||
| } | ||
|
|
||
| return res.status(200).json({ | ||
| message: 'Retrieved all pending reviews', | ||
| result: costing, | ||
| }); | ||
| } catch (err) { | ||
| return res.status(500).json({ error: `Internal Server Error: ${err}` }); | ||
| } |
There was a problem hiding this comment.
I would move around some of these status responses to keep it consistent with the rest of the codebase
| try { | |
| const costing = await minReviewsCosting(min); | |
| if (minReviewsCosting === null) { | |
| return res.status(400).json({ | |
| error: `No reviews found`, | |
| }); | |
| } | |
| return res.status(200).json({ | |
| message: 'Retrieved all pending reviews', | |
| result: costing, | |
| }); | |
| } catch (err) { | |
| return res.status(500).json({ error: `Internal Server Error: ${err}` }); | |
| } | |
| try { | |
| const costing = await minReviewsCosting(min); | |
| if (!costing) { | |
| return res.status(500).json({ error: `Internal Server Error` }); | |
| } | |
| return res.status(200).json({ | |
| message: `Retrieved all pending reviews for a minimum value of ${min}`, | |
| result: costing, | |
| }); | |
| } catch (err) { | |
| return res | |
| .status(400) | |
| .json({ error: `No reviews found` }); | |
| } |
Created a dashboard summary of OpenAI API costs that can be reached at '/admin/costing'. Can choose on the dropdown menu a minimum number of reviews to run the summary on and then will generate the costing estimates for that minimum.
