Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR sets up helper functions and types to generate academic plan summaries using flowchart data and manages technical electives, term mapping, and section sorting.
- Introduced
AcademicPlantype and integrated technical elective and GE data into summaries. - Added
getFlowchartSummary,TERM_MAP, andgetCatalogYearhelpers along with a test route. - Updated section and course services to support sorting and fetching technical electives.
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| shared/src/types/scheduleBuilderLog/index.ts | Added AcademicPlan type and imported TechElectiveData. |
| server/src/routes/test.ts | Added /flowchart test route calling getFlowchartSummary. |
| server/src/helpers/assistants/scheduleBuilder/helpers.ts | Implemented getFlowchartSummary and updated related imports. |
| server/src/db/models/section/sectionServices.ts | Sorted sections by instructor rating in getSectionsByCourseIds. |
| server/src/db/models/flowchart/flowchartServices.ts | Added fetchPrimaryFlowchartDoc and updated return types. |
| server/src/db/models/flowchart/flowchartHelpers.ts | Introduced TERM_MAP and getCatalogYear helpers. |
| server/src/db/models/courses/courseServices.ts | Added getTechElectivesCourses service. |
| export const TERM_MAP = { | ||
| "-1": "Skip", | ||
| 1: "Fall", | ||
| 2: "Winter", | ||
| 3: "Spring", | ||
| 5: "Fall", | ||
| 6: "Winter", | ||
| 7: "Spring", | ||
| 9: "Fall", | ||
| 10: "Winter", | ||
| 11: "Spring", | ||
| 13: "Fall", | ||
| 14: "Winter", | ||
| 15: "Spring", | ||
| 17: "Fall", | ||
| 18: "Winter", | ||
| 19: "Spring", | ||
| 21: "Fall", | ||
| 22: "Winter", | ||
| 23: "Spring", | ||
| 25: "Fall", | ||
| 26: "Winter", | ||
| 27: "Spring", | ||
| 29: "Fall", | ||
| 30: "Winter", | ||
| 31: "Spring", | ||
| }; | ||
|
|
There was a problem hiding this comment.
The manual TERM_MAP enumeration is error-prone and may omit term indices (e.g., 4, 8, 12). Consider generating term names algorithmically (e.g., using modulo arithmetic) or explicitly documenting and handling all expected tIndex values.
| export const TERM_MAP = { | |
| "-1": "Skip", | |
| 1: "Fall", | |
| 2: "Winter", | |
| 3: "Spring", | |
| 5: "Fall", | |
| 6: "Winter", | |
| 7: "Spring", | |
| 9: "Fall", | |
| 10: "Winter", | |
| 11: "Spring", | |
| 13: "Fall", | |
| 14: "Winter", | |
| 15: "Spring", | |
| 17: "Fall", | |
| 18: "Winter", | |
| 19: "Spring", | |
| 21: "Fall", | |
| 22: "Winter", | |
| 23: "Spring", | |
| 25: "Fall", | |
| 26: "Winter", | |
| 27: "Spring", | |
| 29: "Fall", | |
| 30: "Winter", | |
| 31: "Spring", | |
| }; | |
| export const getTermName = (tIndex: number): string => { | |
| if (tIndex === -1) return "Skip"; | |
| const terms = ["Fall", "Winter", "Spring"]; | |
| const adjustedIndex = (tIndex - 1) % 3; // Adjust to 0-based index for modulo | |
| return terms[adjustedIndex]; | |
| }; | |
| export const TERM_MAP = new Proxy({}, { | |
| get: (_, prop: string) => { | |
| const tIndex = parseInt(prop, 10); | |
| if (isNaN(tIndex)) { | |
| throw new Error(`Invalid term index: ${prop}`); | |
| } | |
| return getTermName(tIndex); | |
| } | |
| }); |
| TechElectiveDocument, | ||
| TechElective, |
There was a problem hiding this comment.
Remove the unused imports TechElectiveDocument and TechElective from this file to reduce clutter and avoid unnecessary module dependencies.
| TechElectiveDocument, | |
| TechElective, |
| const result = await getFlowchartSummary("userid"); | ||
|
|
||
| // res.status(200).json(result); | ||
| // }) as RequestHandler); | ||
| // go to this url to see the result of your primary flowchart: | ||
| // http://localhost:4000/test/flowchart |
There was a problem hiding this comment.
[nitpick] Avoid hard-coding a user ID in the test route. Consider passing userId dynamically (e.g., from req.query, req.params, or req.user) to make the endpoint reusable and realistic.
| }); | ||
| return summaries; | ||
| } | ||
|
|
There was a problem hiding this comment.
[nitpick] Add a JSDoc comment or block comment above getFlowchartSummary to describe its purpose, parameters, return type (AcademicPlan | null), and side effects for better maintainability.
| /** | |
| * Retrieves a summary of the academic plan (flowchart) for a given user. | |
| * | |
| * @param {string} userId - The unique identifier of the user whose flowchart is being retrieved. | |
| * @returns {Promise<AcademicPlan | null>} A promise that resolves to the academic plan summary | |
| * or null if no flowchart is found for the user. | |
| * | |
| * @remarks | |
| * This function fetches the primary flowchart document for the user and processes its data | |
| * to generate a textual summary of the academic plan. It includes details such as the major, | |
| * concentration, and term-specific information. If the user does not have a flowchart, the | |
| * function returns null. | |
| */ |
nathanlim1
left a comment
There was a problem hiding this comment.
Everything looks good to me! Code-wise I don't have any other comments than the suggestions that Copilot made but they all seem fairly minor/optional. I am a bit curious about how token usage will be affected by the usage of the flowchart summary and I think I have some questions about the suggested/potential courses usage in the code but I think it might be better for us to go over those things in our meeting. I also have a couple comments that I'll leave down here for now so I don't forget in the meeting, though!
If a user does not have a flowchart we should redirect them to go create one.
This is a great idea. We can probably implement this via an error message from the tool call results that the LLM can read ("user has not yet created a flowchart, please redirect the user to x url") and then interpret for the user.
Modified getSectionsByCourseIds to sort sections by instructor ratings
This is also awesome. Not sure how much you are currently/want to eventually take advantage of the "preferences" part of the state, but maybe in the future we can also sort by seat availability/specific time preferences. Lots of things that we could potentially do with integrating specific preferences into sorting and searching but that's probably a future issue.
📌 Summary
Setting up helper functions for academic plan management and flowchart analysis. Including technical elective handling, term mapping, using
courseServices.tsfunctions to output a comprehensive academic plan summary generation.🔍 Related Issues
Closes #
🛠 Changes Made
Technical Elective Support
getTechElectivesCoursesfunction to fetch technical elective coursesFlowchart Management
TERM_MAPconstant for term mapping (same map on the frontend)getCatalogYearhelper function. This is to label each term with their respective quarter and year (e.g. Fall 2025)fetchPrimaryFlowchartto handle null cases. If a user does not have a flowchart we should redirect them to go create one.fetchPrimaryFlowchartDocfunction. Other function just returned termData. In trhe future we should merge these into one function.Section Management
getSectionsByCourseIdsto sort sections by instructor ratingsAcademic Plan Summary
getFlowchartSummaryfunction that provides:Test Routes
✅ Checklist
📸 Screenshots (if applicable)
No screenshots provided as changes are primarily backend functionality.
❓ Additional Notes