Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3398942cc1
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| @@ -111,6 +111,11 @@ | |||
| </script> | |||
| {% endif %} | |||
|
|
|||
| {% if preloaded_requirements_json %} | |||
| <script> | |||
| window.__TIGERPATH_PRELOADED_REQUIREMENTS__ = {{ preloaded_requirements_json|safe }}; | |||
There was a problem hiding this comment.
The template writes preloaded_requirements_json into a <script> using |safe, but that JSON contains user-controlled external credit names propagated through populate_user_schedule() and check_major/check_degree in views.py. A crafted value containing </script> can break out of the script and execute arbitrary JavaScript on page load (stored XSS for that account). Use Django’s json_script (or equivalent escaping of <, >, and &) instead of raw safe output.
Useful? React with 👍 / 👎.
| api = MobileApp() | ||
|
|
||
| print("[scrape] Fetching registrar token...", flush=True) | ||
| registrar_token = get_registrar_token() |
There was a problem hiding this comment.
The new registrar dependency is fetched unconditionally at startup, and any exception from get_registrar_token() now aborts scrape_parse_semester before any courses are parsed. That means a transient registrar outage or page-format change causes seed-courses to fail for the entire term, whereas course ingestion previously still worked (without dist-area enrichment). Catch this failure and fall back to parsing courses with empty distribution data.
Useful? React with 👍 / 👎.
| if instance.major and instance.major.supported and instance.year: | ||
| schedule = populate_user_schedule(instance.user_schedule) | ||
| try: | ||
| preloaded_requirements.append( | ||
| check_major(instance.major.code, schedule, instance.year) |
There was a problem hiding this comment.
This block eagerly runs check_major/check_degree on every index request, but the frontend still does its normal /api/v1/get_requirements/ fetch and there is no code reading window.__TIGERPATH_PRELOADED_REQUIREMENTS__. The result is duplicate verifier work and larger HTML payloads without the intended first-render speedup. Either consume the preloaded payload during app initialization or skip this server-side computation.
Useful? React with 👍 / 👎.
The hoagie requirements data update (#514) changed category names in the YAML files, breaking the verifier's settled path matching for existing users. This adds a management command to clear stale settled paths and also: - Adds POR/SPA to AB_CONCENTRATIONS (new local YAML files) - Cleans up verifier imports (removes unused `requests`, fixes `pathlib` placement) - Management command also creates Major DB records for POR/SPA Run on production after deploy: docker compose -f docker-compose.prod.yml exec -T web python manage.py clear_settled_paths --apply Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
The hoagie requirements data update (#514) changed category names in the YAML files, breaking the verifier's settled path matching for existing users. This adds a management command to clear stale settled paths and also: - Adds POR/SPA to AB_CONCENTRATIONS (new local YAML files) - Cleans up verifier imports (removes unused `requests`, fixes `pathlib` placement) - Management command also creates Major DB records for POR/SPA Run on production after deploy: docker compose -f docker-compose.prod.yml exec -T web python manage.py clear_settled_paths --apply Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
2 participants
Bundle Hoagie requirements data locally, serve at page load, and fix dist_area scraping
Problem
oldbranch of Princeton-Departmental-Data over HTTP — that data was outdated and the network fetch caused the requirements sidebar to flash in after a delaydist_areaon courses because the modernization PR dropped the per-course/courses/detailscall, and the old MobileApp endpoint now returns 404Changes
Requirements data:
tigerpath/requirements_data/(majors, minors, certificates, degrees)verifier.pywith local file readsindexview and inject into the HTML templateScraper fix:
distribution_area_shortfor each courseTesting
get_requirements→ hard refresh — zero requests on initial loadmake seed-courses— courses now havedist_areapopulated