-
Notifications
You must be signed in to change notification settings - Fork 0
Update thesis content and refresh dependencies #10
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
base: main
Are you sure you want to change the base?
Conversation
- Fix js-yaml prototype pollution vulnerability (CVE-2025-64718) - Add npm override to force js-yaml 4.1.1 for all dependencies - Upgrade ESLint 8.x → 9.x with updated flat config - Upgrade Jest 29.x → 30.x - Upgrade husky 8.x → 9.x with new prepare script format - Upgrade lint-staged 15.x → 16.x - Upgrade markdownlint-cli2 0.18 → 0.20 - Upgrade prettier 3.1 → 3.7 - Upgrade stylelint 16.1 → 16.26 - Upgrade stylelint-config-standard 36 → 37 - Add globals package for ESLint 9.x browser globals - Create stylelint.config.js with vendor file exclusions
Include video embed, defense details, and shared section styling.
Surface the thesis page alongside other top-level links.
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughReplaces a PAT-based CI push with a GitHub App token flow; adds PhD thesis content and MathJax; introduces thesis-specific styles and responsive layout; updates lint config and .gitignore; adds AGENTS.md; minor JS string delimiter and formatting tweaks. Changes
Sequence Diagram(s)sequenceDiagram
participant Action as GitHub Action
participant Repo as Repository (origin)
participant SearchRepo as comphy-search
participant GitHubApp as GitHub App
participant Git as Git (push)
rect rgba(200,220,255,0.5)
Note over Action,Repo: New flow (App token)
Action->>Repo: checkout (fetch-depth preserved)
Action->>SearchRepo: git clone/comphy-search
Action->>GitHubApp: request installation token
GitHubApp-->>Action: returns App token
Action->>SearchRepo: copy `search_db.json` -> working tree
Action->>Git: commit & push using App token
end
rect rgba(255,220,200,0.5)
Note over Action,Repo: Previous flow (PAT)
Action->>Repo: checkout
Action->>SearchRepo: git clone/comphy-search
Action->>Git: commit & push using BYPASS_TOKEN (PAT)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR adds PhD thesis content to the website and performs dependency maintenance. The primary change enables direct access to thesis information through a new navigation link, while supporting updates refresh ESLint configuration and GitHub Actions workflow authentication.
Changes:
- Add PhD thesis navigation link and update documentation to reflect the page is no longer a redirect
- Introduce CSS styling for thesis presentation including info grids and video embeds
- Update ESLint configuration to use the
globalspackage and modernize syntax - Migrate GitHub Actions workflow from PAT to GitHub App token authentication
Reviewed changes
Copilot reviewed 6 out of 6739 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| _layouts/default.html | Adds MathJax support and PhD thesis navigation link |
| assets/css/styles.css | Introduces thesis-specific styling for info grids, video embeds, and responsive layouts |
| assets/js/main.js | Quote style consistency changes (single to double quotes) |
| eslint.config.js | Modernizes configuration using globals package and updates ecmaVersion |
| CLAUDE.md | Updates documentation to reflect phd-thesis.md is now a content page instead of redirect |
| .github/workflows/update-search.yml | Migrates authentication from PAT to GitHub App token |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@CLAUDE.md`:
- Around line 44-52: The documentation claim in phd-thesis.md that "This page is
NOT in the navigation menu" is stale: the site's template _layouts/default.html
currently injects a nav item (<li><a href="{{ site.baseurl }}/phd-thesis/">PhD
Thesis</a></li>), so either remove that nav entry from _layouts/default.html to
keep the page hidden or update the phd-thesis.md/C LAUDE.md content to state
that the PhD Thesis page is included in the site navigation; locate the nav
insertion in _layouts/default.html (around the navigation list) and either
delete or comment it out, or edit phd-thesis.md/CLAUDE.md to accurately describe
the page as present in the navbar and optionally note the previous redirect
behavior.
🧹 Nitpick comments (1)
_layouts/default.html (1)
28-38: Pin MathJax to a specific version and reconsider the$...$inline delimiter.Using
mathjax@3pulls from the latest v3.x release (currently 3.2.2, June 2022), which ensures bug fixes but means your site's math rendering can change without explicit action. MathJax documentation recommends pinning to an exact version in production for reproducibility.Additionally, MathJax deliberately does not enable
$...$as an inline delimiter by default because$appears frequently in plain text (currency, amounts, etc.), leading to accidental math interpretation. The current configuration enables both\(...\)and$...$, which means any literal dollar signs in content must be escaped as\$to avoid being treated as math delimiters. If this dual-delimiter approach is necessary, ensure all content authors understand the escaping requirement.🔧 Suggested fix to pin version
- <script defer src="https://cdn.jsdelivr.net/npm/mathjax@3/es5/tex-svg.js"></script> + <script defer src="https://cdn.jsdelivr.net/npm/mathjax@3.2.2/es5/tex-svg.js"></script>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@AGENTS.md`:
- Line 7: Replace bare URLs in AGENTS.md (e.g., the occurrences of
https://comphy-lab.org/vatsalsy and https://comphy-lab.org and other bare links
referenced in the comment) with proper markdown link syntax or angle-bracket
wrapped links to satisfy markdownlint MD034; update each occurrence to either
[ComPhy Lab](https://comphy-lab.org) style or <https://comphy-lab.org/vatsalsy>
style so the file no longer contains naked URLs.
Summary
Changes Made
Testing