Codex/refactor code quality - #9
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
@copilot resolve the merge conflicts in this pull request |
|
@aeiwz I'm unable to start working on this because of repository rules that prevent me from pushing to the branch:
See the documentation for more details. |
|
@copilot resolve the merge conflicts in this pull request |
|
@aeiwz I'm unable to start working on this because of repository rules that prevent me from pushing to the branch:
See the documentation for more details. |
There was a problem hiding this comment.
Pull request overview
This PR improves the Metbit docs site and Python backend with a focus on annotation security hardening, frontend modernization, and backend refactors (plus a package version bump).
Changes:
- Add sanitization helpers for annotation labels (HTML escaping) and CSV export column naming, plus security-focused tests.
- Refactor
Scalerinternals for clarity/consistency (attribute reset, sparse-centering guard, numeric literal cleanup). - Modernize docs app setup (Next/React upgrade, ESLint flat config,
next/font, theme initialization tweaks) and bump package version.
Reviewed changes
Copilot reviewed 9 out of 40 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
metbit/annotate_peak.py |
Adds annotation/CSV sanitizers and integrates them into annotation add/load and CSV export. |
tests/test_security_annotations.py |
Adds tests for sanitization helpers. |
metbit/scaler.py |
Refactors scaler implementation (sparse-centering checks, reset logic, formatting/literals). |
tests/__pycache__/test_utility.cpython-314-pytest-9.0.2.pyc |
Compiled bytecode added to repo (should not be committed). |
tests/__pycache__/test_spec_norm.cpython-314-pytest-9.0.2.pyc |
Compiled bytecode added to repo (should not be committed). |
tests/__pycache__/test_security_annotations.cpython-314-pytest-9.0.2.pyc |
Compiled bytecode added to repo (should not be committed). |
tests/__pycache__/test_scaler.cpython-314-pytest-9.0.2.pyc |
Compiled bytecode added to repo (should not be committed). |
tests/__pycache__/test_project_name.cpython-314-pytest-9.0.2.pyc |
Compiled bytecode added to repo (should not be committed). |
tests/__pycache__/test_normalise.cpython-314-pytest-9.0.2.pyc |
Compiled bytecode added to repo (should not be committed). |
tests/__pycache__/test_metbit_core.cpython-314-pytest-9.0.2.pyc |
Compiled bytecode added to repo (should not be committed). |
metbit/__pycache__/ui_picky_peak.cpython-314.pyc |
Compiled bytecode added to repo (should not be committed). |
metbit/__pycache__/take_intensity.cpython-314.pyc |
Compiled bytecode added to repo (should not be committed). |
metbit/__pycache__/spec_norm.cpython-314.pyc |
Compiled bytecode added to repo (should not be committed). |
metbit/__pycache__/scaler.cpython-314.pyc |
Compiled bytecode added to repo (should not be committed). |
metbit/__pycache__/pretreatment.cpython-314.pyc |
Compiled bytecode added to repo (should not be committed). |
metbit/__pycache__/pls.cpython-314.pyc |
Compiled bytecode added to repo (should not be committed). |
metbit/__pycache__/peak_processe.cpython-314.pyc |
Compiled bytecode added to repo (should not be committed). |
metbit/__pycache__/pca_ellipse.cpython-314.pyc |
Compiled bytecode added to repo (should not be committed). |
metbit/__pycache__/opls.cpython-314.pyc |
Compiled bytecode added to repo (should not be committed). |
metbit/__pycache__/calibrate.cpython-314.pyc |
Compiled bytecode added to repo (should not be committed). |
metbit/__pycache__/baseline.cpython-314.pyc |
Compiled bytecode added to repo (should not be committed). |
metbit/__pycache__/base.cpython-314.pyc |
Compiled bytecode added to repo (should not be committed). |
metbit/__pycache__/__init__.cpython-314.pyc |
Compiled bytecode added to repo (should not be committed). |
metbit/__pycache__/STOCSY.cpython-314.pyc |
Compiled bytecode added to repo (should not be committed). |
setup.py |
Bumps package version and download URL to 8.7.7. |
metbit/__init__.py |
Bumps __version__ to 8.7.7. |
docs/package.json |
Upgrades Next/React and adjusts lint script to eslint .. |
docs/eslint.config.mjs |
Adds ESLint flat config using Next presets and ignores build outputs. |
docs/next-env.d.ts |
Updates Next TS references for typed routes. |
docs/app/layout.tsx |
Switches Roboto loading to next/font and applies font class to <body>. |
docs/app/components/ThemeToggle.tsx |
Refines theme persistence/init logic via getStoredMode. |
Comments suppressed due to low confidence (1)
metbit/annotate_peak.py:610
- The CSV injection hardening is only applied to the annotation-derived column header (
lbl). Cells coming frommeta_dfare written to CSV unchanged, so a value like=HYPERLINK(...)in metadata can still trigger formula injection when the CSV is opened in a spreadsheet app. Consider sanitizing all string/object cells inmeta_df(and possibly metadata column names) via_sanitize_csv_cellbeforeto_csv.
meta_df = self.meta.copy()
if not isinstance(meta_df, pd.DataFrame):
meta_df = meta_df.to_frame(name=str(meta_df.name or "Meta"))
meta_df = meta_df.loc[self.spectra.index]
out = pd.concat([
meta_df,
intensity_df
], axis=1)
buf = io.StringIO()
out.to_csv(buf, index=False)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 40 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const [mode, setMode] = useState<Mode>(getStoredMode) | ||
|
|
||
| useEffect(() => { | ||
| const saved = (localStorage.getItem('theme') as Mode) || 'system' | ||
| setMode(saved) | ||
| applyTheme(saved) | ||
| const current = getStoredMode() | ||
| applyTheme(current) | ||
|
|
||
| const mq = window.matchMedia('(prefers-color-scheme: dark)') | ||
| const onChange = () => { | ||
| const current = (localStorage.getItem('theme') as Mode) || 'system' | ||
| if (current === 'system') applyTheme('system') | ||
| if (getStoredMode() === 'system') applyTheme('system') | ||
| } |
There was a problem hiding this comment.
useState(getStoredMode) initializes mode during SSR as 'system', and the useEffect now applies the stored theme but never calls setMode(current). This can leave the toggle UI showing the wrong active button until the user clicks (theme applied != state). Update the effect to also sync state (and consider updating state on matchMedia change when mode is system).
There was a problem hiding this comment.
@copilot apply changes based on this feedback @copilot
| "next": "^16.2.3", | ||
| "react": "^19.2.4", | ||
| "react-dom": "^19.2.4", | ||
| "react-icons": "^5.6.0" | ||
| }, | ||
| "devDependencies": { | ||
| "@types/node": "^20.11.30", | ||
| "@types/react": "^18.2.79", | ||
| "eslint": "^8.57.0", | ||
| "eslint-config-next": "^16.0.6", | ||
| "typescript": "^5.4.5", | ||
| "@mdx-js/loader": "^3.0.1", | ||
| "@next/mdx": "^14.2.5", | ||
| "@mdx-js/react": "^3.0.1" | ||
| "@mdx-js/loader": "^3.1.1", | ||
| "@mdx-js/react": "^3.1.1", | ||
| "@next/mdx": "^16.2.3", | ||
| "@types/node": "^25.5.2", | ||
| "@types/react": "^19.2.14", | ||
| "eslint": "^10.2.0", | ||
| "eslint-config-next": "^16.2.3", | ||
| "typescript": "^6.0.2" |
There was a problem hiding this comment.
These dependency bumps (Next 16 / ESLint 10 / TS 6) also raise the minimum supported Node.js version (e.g. next@16.2.3 requires Node >=20.9 and eslint@10.2.0 requires Node ^20.19 || ^22.13 || >=24 per the lockfile). Please declare/enforce the required Node version for the docs site (e.g. via engines in package.json and/or an .nvmrc), otherwise installs/lint/build can break on existing environments/CI.
|
@aeiwz I'm unable to start working on this because of repository rules that prevent me from pushing to the branch:
See the documentation for more details. |
1 similar comment
|
@aeiwz I'm unable to start working on this because of repository rules that prevent me from pushing to the branch:
See the documentation for more details. |
This pull request introduces several improvements across both the frontend documentation site and the Python backend, focusing on security, maintainability, and user experience. The most significant changes include implementing input sanitization to prevent XSS and CSV injection in annotation features, updating dependencies and linting configuration for the Next.js frontend, and refactoring the scaler utility for clarity and robustness.
Security enhancements for annotation features:
_sanitize_annotation_textto escape HTML in annotation labels and_sanitize_csv_cellto prevent spreadsheet formula injection in CSV exports withinmetbit/annotate_peak.py. These functions are now used when adding annotations and exporting intensity tables, reducing the risk of XSS and CSV injection attacks. [1] [2] [3] [4]tests/test_security_annotations.pyto verify the effectiveness of the sanitization functions.Frontend dependency and configuration updates:
next,react,react-dom,react-icons, and related dev dependencies inpackage.jsonto their latest major versions, and updated the lint script to useeslint .instead ofnext lint.eslint-config-nextanddefineConfigindocs/eslint.config.mjsfor improved code quality and compatibility.Frontend UI improvements:
next/fontAPI for loading the Roboto font inlayout.tsx, removing manual<link>tags and applying the font via className for better performance and maintainability. [1] [2]ThemeToggle.tsxby introducing agetStoredModefunction and simplifying the logic in the component. [1] [2]Python backend refactoring and bug fixes:
metbit/scaler.pyfor clarity: added a static method to handle sparse matrix centering errors, improved attribute management in_reset, and standardized numeric literals. Also improved handling of sparse matrices in scaling methods. [1] [2] [3] [4] [5]Version updates:
8.7.7in both__init__.pyandsetup.py, and updated the download URL accordingly. [1] [2]