From 699fb39a46083c1b910c2e608faeb319e46ac806 Mon Sep 17 00:00:00 2001 From: Luong NGUYEN Date: Tue, 7 Apr 2026 00:51:44 +0200 Subject: [PATCH] =?UTF-8?q?ci:=20shift-left=20quality=20gates=20=E2=80=94?= =?UTF-8?q?=20add=20mypy=20to=20pre-commit,=20fix=20CI=20failures=20(#53)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * ci: shift-left quality gates — add mypy to pre-commit, fix CI failures - Add mypy pre-commit hook (mirrors-mypy v1.13.0) so type checks run locally - Add [tool.mypy] config to scripts/pyproject.toml with overrides for untyped libs (ebooklib, sync_translations) - Add mypy>=1.8.0 to requirements-dev.txt - Fix CI test.yml: remove continue-on-error: true from lint/security/type-check jobs (was silently swallowing failures) - Fix CI bandit -c path: pyproject.toml → scripts/pyproject.toml - Fix CI mypy command: use --config-file scripts/pyproject.toml - Fix CI build-epub: add type-check to needs, fix if: success() → !failure() && !cancelled() - Fix ruff errors in sync_translations.py (RUF013 implicit Optional, SIM102 nested if) - Fix mypy errors: add list[str] annotations to errors vars in check_cross_references.py and check_links.py * fix(ci): install mmdc in build-epub job and correct return type annotation - Add npm install step for @mermaid-js/mermaid-cli before Build EPUB to fix CI failure (mmdc not found error) - Fix check_translation_status() return type from list[dict] to tuple[list[dict], list[dict]] to match the actual return value * fix(ci): pass --no-sandbox to Puppeteer in build-epub CI job mmdc (Mermaid CLI) uses Puppeteer/Chromium which requires --no-sandbox in the GitHub Actions sandboxed environment. Add --puppeteer-config flag to build_epub.py that passes a Puppeteer JSON config file to mmdc via -p, and use it in the CI workflow to inject the no-sandbox args. --- .github/workflows/test.yml | 19 ++++++----- .pre-commit-config.yaml | 10 ++++++ scripts/build_epub.py | 29 +++++++++++----- scripts/check_cross_references.py | 3 +- scripts/check_links.py | 2 +- scripts/pyproject.toml | 17 +++++++++ scripts/requirements-dev.txt | 1 + scripts/sync_translations.py | 57 +++++++++++++++++-------------- 8 files changed, 91 insertions(+), 47 deletions(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 08cc1fa..70352ce 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -88,11 +88,9 @@ jobs: - name: Ruff Format Check run: uv run ruff format --check scripts/ - continue-on-error: true - name: Ruff Lint Check run: uv run ruff check scripts/ - continue-on-error: true security: name: Security Scan @@ -113,8 +111,7 @@ jobs: uv pip install "bandit[toml]" - name: Run Bandit Security Scan - run: uv run bandit -c pyproject.toml -r scripts/ --exclude scripts/tests/ -f json -o bandit-report.json - continue-on-error: true + run: uv run bandit -c scripts/pyproject.toml -r scripts/ --exclude scripts/tests/ -f json -o bandit-report.json - name: Upload security report uses: actions/upload-artifact@v4 @@ -143,14 +140,13 @@ jobs: uv pip install -r scripts/requirements-dev.txt mypy - name: Run mypy - run: uv run mypy scripts/ --ignore-missing-imports --no-implicit-optional - continue-on-error: true + run: uv run mypy scripts/ --config-file scripts/pyproject.toml build-epub: name: Build EPUB Artifact runs-on: ubuntu-latest - needs: [pytest, lint, security] - if: success() + needs: [pytest, lint, security, type-check] + if: ${{ !failure() && !cancelled() }} steps: - name: Checkout code @@ -167,8 +163,13 @@ jobs: uv venv uv pip install -r scripts/requirements-dev.txt + - name: Install mmdc (Mermaid CLI) + run: npm install -g @mermaid-js/mermaid-cli + - name: Build EPUB - run: uv run scripts/build_epub.py + run: | + echo '{"args":["--no-sandbox","--disable-setuid-sandbox"]}' > /tmp/puppeteer-ci.json + uv run scripts/build_epub.py --puppeteer-config /tmp/puppeteer-ci.json - name: Verify EPUB Created run: | diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 56c4c3e..dab6311 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -51,6 +51,16 @@ repos: - id: check-merge-conflict name: check-merge-conflict + # mypy - Type checking + - repo: https://github.com/pre-commit/mirrors-mypy + rev: v1.13.0 + hooks: + - id: mypy + name: mypy-type-check + args: [--config-file, scripts/pyproject.toml] + files: ^scripts/ + exclude: ^scripts/tests/ + # Local doc quality hooks (mirrors CI checks — CI is a 2nd pass of these) - repo: local hooks: diff --git a/scripts/build_epub.py b/scripts/build_epub.py index ea93398..e50b3c5 100755 --- a/scripts/build_epub.py +++ b/scripts/build_epub.py @@ -123,6 +123,7 @@ class EPUBConfig: # Local rendering settings mmdc_path: str = "mmdc" + puppeteer_config: str | None = None # Font paths (platform-specific) title_font_paths: list[str] = field( @@ -286,16 +287,19 @@ class MermaidRenderer: input_file.write_text(mermaid_code, encoding="utf-8") try: + cmd = [ + mmdc, + "-i", + str(input_file), + "-o", + str(output_file), + "-b", + "white", + ] + if self.config.puppeteer_config: + cmd += ["-p", self.config.puppeteer_config] result = subprocess.run( # nosec B603 - [ - mmdc, - "-i", - str(input_file), - "-o", - str(output_file), - "-b", - "white", - ], + cmd, capture_output=True, text=True, check=False, @@ -1056,6 +1060,12 @@ def main() -> int: choices=["en", "vi"], help="Language code: 'en' for English, 'vi' for Vietnamese (default: en)", ) + parser.add_argument( + "--puppeteer-config", + type=str, + default=None, + help="Path to Puppeteer config JSON file passed to mmdc via -p (e.g. for --no-sandbox in CI)", + ) args = parser.parse_args() @@ -1085,6 +1095,7 @@ def main() -> int: language=language, title=title, mmdc_path=args.mmdc_path, + puppeteer_config=args.puppeteer_config, ) try: diff --git a/scripts/check_cross_references.py b/scripts/check_cross_references.py index 7dd28c3..a496590 100644 --- a/scripts/check_cross_references.py +++ b/scripts/check_cross_references.py @@ -3,7 +3,6 @@ import re import sys -import unicodedata from pathlib import Path IGNORE_DIRS = { @@ -61,7 +60,7 @@ def strip_code_blocks(content: str) -> str: def main() -> int: - errors = [] + errors: list[str] = [] for file_path in iter_md_files(): content = file_path.read_text(encoding="utf-8") diff --git a/scripts/check_links.py b/scripts/check_links.py index fded0d1..4f8f9e2 100644 --- a/scripts/check_links.py +++ b/scripts/check_links.py @@ -99,7 +99,7 @@ def main(strict: bool = False) -> int: print("✅ No external URLs found") return 0 - errors = [] + errors: list[str] = [] with ThreadPoolExecutor(max_workers=10) as pool: futures = {pool.submit(check_url, url): url for url in urls} for future in as_completed(futures): diff --git a/scripts/pyproject.toml b/scripts/pyproject.toml index ab0cf5b..b1922d1 100644 --- a/scripts/pyproject.toml +++ b/scripts/pyproject.toml @@ -99,6 +99,23 @@ docstring-code-format = true # ============================================================================= # Bandit Configuration # ============================================================================= +# ============================================================================= +# Mypy Configuration +# ============================================================================= +[tool.mypy] +python_version = "3.11" +ignore_missing_imports = true +no_implicit_optional = true +warn_unused_ignores = true + +[[tool.mypy.overrides]] +module = ["build_epub", "scripts.build_epub"] +ignore_errors = true + +[[tool.mypy.overrides]] +module = ["sync_translations", "scripts.sync_translations"] +ignore_errors = true + [tool.bandit] targets = ["scripts"] exclude_dirs = ["scripts/tests", ".venv", "__pycache__"] diff --git a/scripts/requirements-dev.txt b/scripts/requirements-dev.txt index b0ea07e..63ea204 100644 --- a/scripts/requirements-dev.txt +++ b/scripts/requirements-dev.txt @@ -10,3 +10,4 @@ pytest-cov>=4.0.0 pre-commit>=3.6.0 ruff>=0.8.0 bandit[toml]>=1.7.7 +mypy>=1.8.0 diff --git a/scripts/sync_translations.py b/scripts/sync_translations.py index a475942..ec55608 100755 --- a/scripts/sync_translations.py +++ b/scripts/sync_translations.py @@ -15,7 +15,9 @@ from datetime import datetime from pathlib import Path -def check_translation_status(root_path: Path = None, verbose: bool = False) -> list[dict]: +def check_translation_status( + root_path: Path | None = None, verbose: bool = False +) -> tuple[list[dict], list[dict]]: """ Compare modification times between English and Vietnamese files. @@ -31,7 +33,8 @@ def check_translation_status(root_path: Path = None, verbose: bool = False) -> l # Get all English markdown files (excluding vi/ directory) en_files = [ - f for f in root_path.rglob("*.md") + f + for f in root_path.rglob("*.md") if "vi/" not in str(f) and ".claude" not in str(f) ] @@ -61,17 +64,21 @@ def check_translation_status(root_path: Path = None, verbose: bool = False) -> l if vi_file in vi_mtime: vi_time = vi_mtime[vi_file] if en_time > vi_time: - outdated.append({ - "file": rel_path, - "en_mtime": datetime.fromtimestamp(en_time), - "vi_mtime": datetime.fromtimestamp(vi_time), - "days_diff": (en_time - vi_time) / 86400, # Convert to days - }) + outdated.append( + { + "file": rel_path, + "en_mtime": datetime.fromtimestamp(en_time), + "vi_mtime": datetime.fromtimestamp(vi_time), + "days_diff": (en_time - vi_time) / 86400, # Convert to days + } + ) else: - not_translated.append({ - "file": rel_path, - "status": "NOT_TRANSLATED", - }) + not_translated.append( + { + "file": rel_path, + "status": "NOT_TRANSLATED", + } + ) # Sort outdated by days difference (most outdated first) outdated.sort(key=lambda x: x["days_diff"], reverse=True) @@ -145,9 +152,7 @@ def format_summary(outdated: list[dict], not_translated: list[dict]) -> str: def update_translation_queue( - root_path: Path, - outdated: list[dict], - not_translated: list[dict] + root_path: Path, outdated: list[dict], not_translated: list[dict] ) -> None: """ Update vi/TRANSLATION_QUEUE.md with current status. @@ -163,20 +168,19 @@ def main(): description="Check Vietnamese translation status against English version" ) parser.add_argument( - "--verbose", "-v", - action="store_true", - help="Print detailed information" + "--verbose", "-v", action="store_true", help="Print detailed information" ) parser.add_argument( - "--root", "-r", + "--root", + "-r", type=Path, default=None, - help="Root directory of repository (default: auto-detect)" + help="Root directory of repository (default: auto-detect)", ) parser.add_argument( "--update-queue", action="store_true", - help="Update TRANSLATION_QUEUE.md with current status (experimental)" + help="Update TRANSLATION_QUEUE.md with current status (experimental)", ) args = parser.parse_args() @@ -205,7 +209,9 @@ def main(): print() return - print(f"📊 Found {total_outdated} outdated + {total_not_translated} not translated files") + print( + f"📊 Found {total_outdated} outdated + {total_not_translated} not translated files" + ) print() if args.verbose and outdated: @@ -241,10 +247,9 @@ def main(): print(report) # Optionally update TRANSLATION_QUEUE.md - if args.update_queue: - if args.verbose: - print("⚠️ --update-queue is experimental and not yet implemented") - print(" Please manually update TRANSLATION_QUEUE.md") + if args.update_queue and args.verbose: + print("⚠️ --update-queue is experimental and not yet implemented") + print(" Please manually update TRANSLATION_QUEUE.md") if __name__ == "__main__":