codeant-ai-for-open-source[bot] commented on code in PR #39724: URL: https://github.com/apache/superset/pull/39724#discussion_r3870293167
########## scripts/compile_po.py: ########## @@ -0,0 +1,203 @@ +#!/usr/bin/env python3 +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +# This script is a cross-platform Python equivalent of po2json.sh. +# It generates .json files from .po translation files used by the frontend. + +from __future__ import annotations + +import glob +import os +import shutil +import subprocess +import sys +from concurrent.futures import ThreadPoolExecutor, as_completed + +_SHELL = os.name == "nt" + + +def run_command(command: list[str], cwd: str | None = None, timeout: int = 120) -> int: + try: + result = subprocess.run( # noqa: S603 + command, text=True, shell=_SHELL, check=False, cwd=cwd, timeout=timeout + ) + return result.returncode + except (subprocess.TimeoutExpired, FileNotFoundError, OSError): + return 1 + + +def find_command(names: list[str]) -> str | None: + for name in names: + if path := shutil.which(name): + return path + return None + + +def install_npm_packages(npm_cmd: str, root_dir: str, packages: list[str]) -> bool: + rc = run_command( + [npm_cmd, "install", "--no-save", "--prefer-offline", *packages], + cwd=root_dir, + ) + return rc == 0 + + +def convert_po_file( + po_file: str, frontend_trans_dir: str, po2json_cmd: list[str] +) -> tuple[bool, str, str]: + locale_rel = os.path.relpath( + po_file, start=os.path.dirname(os.path.dirname(po_file)) + ) + json_dest = os.path.join( + frontend_trans_dir, os.path.splitext(locale_rel)[0] + ".json" + ) Review Comment: **Suggestion:** `locale_rel` is computed relative to the locale directory, so `fr/LC_MESSAGES/messages.po` and `zh/LC_MESSAGES/messages.po` both map to `LC_MESSAGES/messages.json`. Parallel conversions therefore overwrite each other and leave only one locale's translation data. Compute the relative path from the common translations root so the locale component is preserved. [logic error] <details> <summary><b>Severity Level:</b> Critical 🚨</summary> ```mdx - ❌ Multiple locale translations overwrite one JSON output. - ❌ Frontend users receive missing or incorrect translations. - ⚠️ Generated files no longer preserve locale directory structure. ``` </details> [](https://docs.codeant.ai/cli/resolve-pr-comments-skill) [](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=3cf10ba7f0b54d929adacc72f54f2f0c&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=3cf10ba7f0b54d929adacc72f54f2f0c&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) <details> <summary><b>Prompt for AI Agent 🤖 </b></summary> ```mdx This is a comment left during a code review. **Path:** scripts/compile_po.py **Line:** 62:67 **Comment:** *Logic Error: `locale_rel` is computed relative to the locale directory, so `fr/LC_MESSAGES/messages.po` and `zh/LC_MESSAGES/messages.po` both map to `LC_MESSAGES/messages.json`. Parallel conversions therefore overwrite each other and leave only one locale's translation data. Compute the relative path from the common translations root so the locale component is preserved. Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise. Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix ``` </details> <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39724&comment_hash=3d0efa9765a98e05b2257a3ed897b6063c4dd56c0021230e076d32725e6276ea&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39724&comment_hash=3d0efa9765a98e05b2257a3ed897b6063c4dd56c0021230e076d32725e6276ea&reaction=dislike'>👎</a> ########## scripts/compile_po.py: ########## @@ -0,0 +1,203 @@ +#!/usr/bin/env python3 +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +# This script is a cross-platform Python equivalent of po2json.sh. +# It generates .json files from .po translation files used by the frontend. + +from __future__ import annotations + +import glob +import os +import shutil +import subprocess +import sys +from concurrent.futures import ThreadPoolExecutor, as_completed + +_SHELL = os.name == "nt" + + +def run_command(command: list[str], cwd: str | None = None, timeout: int = 120) -> int: + try: + result = subprocess.run( # noqa: S603 + command, text=True, shell=_SHELL, check=False, cwd=cwd, timeout=timeout + ) + return result.returncode + except (subprocess.TimeoutExpired, FileNotFoundError, OSError): + return 1 + + +def find_command(names: list[str]) -> str | None: + for name in names: + if path := shutil.which(name): + return path + return None + + +def install_npm_packages(npm_cmd: str, root_dir: str, packages: list[str]) -> bool: + rc = run_command( + [npm_cmd, "install", "--no-save", "--prefer-offline", *packages], + cwd=root_dir, + ) + return rc == 0 + + +def convert_po_file( + po_file: str, frontend_trans_dir: str, po2json_cmd: list[str] +) -> tuple[bool, str, str]: + locale_rel = os.path.relpath( + po_file, start=os.path.dirname(os.path.dirname(po_file)) + ) + json_dest = os.path.join( + frontend_trans_dir, os.path.splitext(locale_rel)[0] + ".json" + ) + os.makedirs(os.path.dirname(json_dest), exist_ok=True) + + cmd = [ + *po2json_cmd, + "--domain", + "superset", + "--format", + "jed1.x", + "--fuzzy", + po_file, + json_dest, + ] + if (rc := run_command(cmd, timeout=60)) != 0: + return False, po_file, f"po2json failed (rc={rc})" + return True, po_file, "" + + +def compile_translations() -> int: + root_dir = os.path.abspath( + os.path.join(os.path.dirname(os.path.abspath(__file__)), "..") + ) + translations_dir = os.path.join(root_dir, "superset", "translations") + frontend_trans_dir = os.path.join( + root_dir, "superset-frontend", "src", "translations" + ) + + try: + import babel # noqa: F401 + except ImportError: + print("ERROR: Babel is not installed. Run: pip install babel", file=sys.stderr) + return 1 + + npm_cmd = find_command(["npm"]) + npx_cmd = find_command(["npx"]) + if not npm_cmd or not npx_cmd: + print("ERROR: Node.js/npm/npx not found in PATH.", file=sys.stderr) + return 1 + + if not os.path.isdir(translations_dir): + print( + f"ERROR: translations directory not found: {translations_dir}", + file=sys.stderr, + ) + return 1 + + print("Step 1: Compiling .po files with pybabel...") + rc = run_command( + [ + sys.executable, + "-m", + "babel.messages.frontend", + "compile", + "-d", + translations_dir, + ], + cwd=root_dir, + ) + if rc != 0: + print("ERROR: pybabel compile failed.", file=sys.stderr) + return 1 + + node_modules_bin = os.path.join(root_dir, "node_modules", ".bin") + po2json_bin = os.path.join(node_modules_bin, "po2json") + prettier_bin = os.path.join(node_modules_bin, "prettier") + + print("Step 2: Installing npm packages (po2json, prettier)...") + packages_needed = [] + if not os.path.isfile(po2json_bin): + packages_needed.append("po2json") + if not os.path.isfile(prettier_bin): + packages_needed.append("prettier") + + if packages_needed and not install_npm_packages( + npm_cmd, root_dir, packages_needed + ): + print("WARNING: npm install failed, falling back to npx.", file=sys.stderr) + + if os.path.isfile(po2json_bin): + po2json_cmd: list[str] = [po2json_bin] + else: + po2json_cmd = [npx_cmd, "-y", "po2json"] + + if os.path.isfile(prettier_bin): + prettier_cmd: list[str] | None = [prettier_bin] + else: + prettier_cmd = [npx_cmd, "-y", "prettier"] if npx_cmd else None + + po_files = glob.glob( + os.path.join(translations_dir, "**", "*.po"), recursive=True + ) + print(f"Step 3: Converting {len(po_files)} .po files to JSON (frontend path)...") + + failures: list[str] = [] + max_workers = min(8, (os.cpu_count() or 1) * 2) + with ThreadPoolExecutor(max_workers=max_workers) as executor: + futures = { + executor.submit(convert_po_file, f, frontend_trans_dir, po2json_cmd): f + for f in po_files + } + for future in as_completed(futures): + ok, po_path, err = future.result() + if not ok: + print(f" FAILED: {po_path} - {err}", file=sys.stderr) + failures.append(po_path) + else: + print(f" OK: {po_path}") + + if failures: + print( + f"\nERROR: {len(failures)} file(s) failed conversion:", file=sys.stderr + ) + for f in failures: + print(f" - {f}", file=sys.stderr) + return 1 + + json_files = glob.glob( + os.path.join(frontend_trans_dir, "**", "*.json"), recursive=True + ) + if json_files and prettier_cmd: + print(f"Step 4: Running prettier on {len(json_files)} JSON files...") + if ( + run_command( + [*prettier_cmd, "--write", *json_files], + cwd=root_dir, + timeout=300, + ) + != 0 + ): + print("WARNING: prettier step failed.", file=sys.stderr) + + print("\nPipeline completed successfully!") + return 0 Review Comment: **Suggestion:** A nonzero Prettier exit status is only logged as a warning, after which the script prints success and returns zero. CI or callers therefore cannot detect that the formatting stage failed, so the pipeline reports successful completion with potentially unformatted generated files. [incorrect condition logic] <details> <summary><b>Severity Level:</b> Minor 🧹</summary> ```mdx - ⚠️ CI cannot detect failed JSON formatting. - ⚠️ Generated translation files may remain unformatted. - ⚠️ The CLI reports success after an incomplete pipeline. ``` </details> [](https://docs.codeant.ai/cli/resolve-pr-comments-skill) [](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=372c759d8bdd496397c1ab6d992e7b32&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=372c759d8bdd496397c1ab6d992e7b32&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) <details> <summary><b>Prompt for AI Agent 🤖 </b></summary> ```mdx This is a comment left during a code review. **Path:** scripts/compile_po.py **Line:** 189:199 **Comment:** *Incorrect Condition Logic: A nonzero Prettier exit status is only logged as a warning, after which the script prints success and returns zero. CI or callers therefore cannot detect that the formatting stage failed, so the pipeline reports successful completion with potentially unformatted generated files. Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise. Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix ``` </details> <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39724&comment_hash=b3598f485c0fb8d87c0c8d55ae1ceb2b6193d39ffa167fa952f487f56d7eb03d&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39724&comment_hash=b3598f485c0fb8d87c0c8d55ae1ceb2b6193d39ffa167fa952f487f56d7eb03d&reaction=dislike'>👎</a> ########## scripts/compile_po.py: ########## @@ -0,0 +1,203 @@ +#!/usr/bin/env python3 +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +# This script is a cross-platform Python equivalent of po2json.sh. +# It generates .json files from .po translation files used by the frontend. + +from __future__ import annotations + +import glob +import os +import shutil +import subprocess +import sys +from concurrent.futures import ThreadPoolExecutor, as_completed + +_SHELL = os.name == "nt" + + +def run_command(command: list[str], cwd: str | None = None, timeout: int = 120) -> int: + try: + result = subprocess.run( # noqa: S603 + command, text=True, shell=_SHELL, check=False, cwd=cwd, timeout=timeout + ) + return result.returncode + except (subprocess.TimeoutExpired, FileNotFoundError, OSError): + return 1 + + +def find_command(names: list[str]) -> str | None: + for name in names: + if path := shutil.which(name): + return path + return None + + +def install_npm_packages(npm_cmd: str, root_dir: str, packages: list[str]) -> bool: + rc = run_command( + [npm_cmd, "install", "--no-save", "--prefer-offline", *packages], + cwd=root_dir, + ) + return rc == 0 + + +def convert_po_file( + po_file: str, frontend_trans_dir: str, po2json_cmd: list[str] +) -> tuple[bool, str, str]: + locale_rel = os.path.relpath( + po_file, start=os.path.dirname(os.path.dirname(po_file)) + ) + json_dest = os.path.join( + frontend_trans_dir, os.path.splitext(locale_rel)[0] + ".json" + ) + os.makedirs(os.path.dirname(json_dest), exist_ok=True) + + cmd = [ + *po2json_cmd, + "--domain", + "superset", + "--format", + "jed1.x", + "--fuzzy", + po_file, + json_dest, + ] + if (rc := run_command(cmd, timeout=60)) != 0: + return False, po_file, f"po2json failed (rc={rc})" + return True, po_file, "" + + +def compile_translations() -> int: + root_dir = os.path.abspath( + os.path.join(os.path.dirname(os.path.abspath(__file__)), "..") + ) + translations_dir = os.path.join(root_dir, "superset", "translations") + frontend_trans_dir = os.path.join( + root_dir, "superset-frontend", "src", "translations" + ) + + try: + import babel # noqa: F401 + except ImportError: + print("ERROR: Babel is not installed. Run: pip install babel", file=sys.stderr) + return 1 + + npm_cmd = find_command(["npm"]) + npx_cmd = find_command(["npx"]) + if not npm_cmd or not npx_cmd: + print("ERROR: Node.js/npm/npx not found in PATH.", file=sys.stderr) + return 1 + + if not os.path.isdir(translations_dir): + print( + f"ERROR: translations directory not found: {translations_dir}", + file=sys.stderr, + ) + return 1 + + print("Step 1: Compiling .po files with pybabel...") + rc = run_command( + [ + sys.executable, + "-m", + "babel.messages.frontend", + "compile", + "-d", + translations_dir, + ], + cwd=root_dir, + ) + if rc != 0: + print("ERROR: pybabel compile failed.", file=sys.stderr) + return 1 + + node_modules_bin = os.path.join(root_dir, "node_modules", ".bin") + po2json_bin = os.path.join(node_modules_bin, "po2json") + prettier_bin = os.path.join(node_modules_bin, "prettier") Review Comment: **Suggestion:** The npm binaries are searched for and installed under the repository root, but the declared `po2json` and `prettier` dependencies and normal `node_modules` installation live under `superset-frontend`. This ignores an existing frontend toolchain and can fall back to network-based `npx` execution when the root install fails, causing the script to fail in offline or locked-down environments despite the required packages already being installed. [api mismatch] <details> <summary><b>Severity Level:</b> Major ⚠️</summary> ```mdx - ❌ Standard frontend installs are not reused. - ❌ Offline translation builds can fail unexpectedly. - ⚠️ CI performs unnecessary network-dependent package resolution. ``` </details> [](https://docs.codeant.ai/cli/resolve-pr-comments-skill) [](https://app.codeant.ai/fix-in-ide?tool=cursor&prompt_id=9c04380c5ef9465981282daab585aec1&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) [](https://app.codeant.ai/fix-in-ide?tool=vscode-claude&prompt_id=9c04380c5ef9465981282daab585aec1&service=github&base_url=https%3A%2F%2Fgithub.com&org=apache&repo=apache%2Fsuperset) <details> <summary><b>Prompt for AI Agent 🤖 </b></summary> ```mdx This is a comment left during a code review. **Path:** scripts/compile_po.py **Line:** 129:131 **Comment:** *Api Mismatch: The npm binaries are searched for and installed under the repository root, but the declared `po2json` and `prettier` dependencies and normal `node_modules` installation live under `superset-frontend`. This ignores an existing frontend toolchain and can fall back to network-based `npx` execution when the root install fails, causing the script to fail in offline or locked-down environments despite the required packages already being installed. Validate the correctness of the flagged issue. If correct, How can I resolve this? If you propose a fix, implement it and please make it concise. Once fix is implemented, also check other comments on the same PR, and ask user if the user wants to fix the rest of the comments as well. if said yes, then fetch all the comments validate the correctness and implement a minimal fix ``` </details> <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39724&comment_hash=b4233471caeea7fcedea1f38a03c1866ab90722f7fb15ad90beeaae3b0260657&reaction=like'>👍</a> | <a href='https://app.codeant.ai/feedback?pr_url=https%3A%2F%2Fgithub.com%2Fapache%2Fsuperset%2Fpull%2F39724&comment_hash=b4233471caeea7fcedea1f38a03c1866ab90722f7fb15ad90beeaae3b0260657&reaction=dislike'>👎</a> -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
