Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions .github/workflows/validate-scripts-json.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
name: Validate scripts.json

on:
push:
branches: [main]
paths:
- 'scripts.json'
- 'tools/validate_scripts.py'
- '.github/workflows/validate-scripts-json.yml'
pull_request:
paths:
- 'scripts.json'
- 'tools/validate_scripts.py'
- '.github/workflows/validate-scripts-json.yml'

jobs:
validate:
runs-on: ubuntu-latest
steps:
- uses: actions/checkout@v6
- uses: astral-sh/[email protected]
- name: Validate scripts.json
run: uv run tools/validate_scripts.py --scripts-json scripts.json
118 changes: 118 additions & 0 deletions tools/validate_scripts.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
#!/usr/bin/env python3
"""
Validate scripts.json against the expected schema.

Exit 0 = valid, Exit 1 = errors found.
"""

import argparse
import json
import sys
from pathlib import Path

KNOWN_CATEGORIES = {
"Audio & Media",
"Flight Controller Config",
"Games & Fun",
"GPS & Mapping",
"Logging & Analysis",
"Radio Tools",
"Telemetry & Widgets",
}

REQUIRED_FIELDS = ["name", "category", "description", "infourl", "images", "tags"]
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
STRING_FIELDS = ["name", "category", "description", "infourl"]


def load_and_parse(path: Path) -> list:
if not path.exists():
print(f"Error: {path} not found", file=sys.stderr)
sys.exit(1)
try:
with open(path, encoding="utf-8") as f:
data = json.load(f)
except json.JSONDecodeError as e:
print(f"Error: invalid JSON in {path}: {e}", file=sys.stderr)
sys.exit(1)
Comment thread
coderabbitai[bot] marked this conversation as resolved.
if not isinstance(data, list):
print(f"Error: {path} must be a JSON array", file=sys.stderr)
sys.exit(1)
return data


def validate(data: list) -> list[str]:
errors = []
seen_names: dict[str, int] = {}

for i, entry in enumerate(data):
prefix = f"Entry {i}"

if not isinstance(entry, dict):
errors.append(f"{prefix}: not an object")
continue

for field in REQUIRED_FIELDS:
if field not in entry:
errors.append(f"{prefix}: missing required field '{field}'")

for field in STRING_FIELDS:
if field in entry:
if not isinstance(entry[field], str) or not entry[field].strip():
errors.append(f"{prefix}: '{field}' must be a non-empty string")

category = entry.get("category")
if isinstance(category, str) and category.strip() and category not in KNOWN_CATEGORIES:
errors.append(
f"{prefix}: unknown category '{category}'"
f" (known: {sorted(KNOWN_CATEGORIES)})"
)

infourl = entry.get("infourl")
if isinstance(infourl, str) and infourl.strip():
if not (infourl.startswith("http://") or infourl.startswith("https://")):
errors.append(f"{prefix}: 'infourl' must start with http:// or https://")

if "images" in entry and not isinstance(entry["images"], list):
errors.append(f"{prefix}: 'images' must be a list")

if "tags" in entry and not isinstance(entry["tags"], list):
errors.append(f"{prefix}: 'tags' must be a list")
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot May 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Enforce non-empty images and tags lists.

Line 75 and Line 78 validate only list type, but required fields are supposed to be non-empty as well. Empty lists currently pass.

💡 Proposed fix
-        if "images" in entry and not isinstance(entry["images"], list):
-            errors.append(f"{prefix}: 'images' must be a list")
+        if "images" in entry:
+            if not isinstance(entry["images"], list) or not entry["images"]:
+                errors.append(f"{prefix}: 'images' must be a non-empty list")
 
-        if "tags" in entry and not isinstance(entry["tags"], list):
-            errors.append(f"{prefix}: 'tags' must be a list")
+        if "tags" in entry:
+            if not isinstance(entry["tags"], list) or not entry["tags"]:
+                errors.append(f"{prefix}: 'tags' must be a non-empty list")
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if "images" in entry and not isinstance(entry["images"], list):
errors.append(f"{prefix}: 'images' must be a list")
if "tags" in entry and not isinstance(entry["tags"], list):
errors.append(f"{prefix}: 'tags' must be a list")
if "images" in entry:
if not isinstance(entry["images"], list) or not entry["images"]:
errors.append(f"{prefix}: 'images' must be a non-empty list")
if "tags" in entry:
if not isinstance(entry["tags"], list) or not entry["tags"]:
errors.append(f"{prefix}: 'tags' must be a non-empty list")
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tools/validate_scripts.py` around lines 75 - 79, The current validation for
entry["images"] and entry["tags"] only checks type but allows empty lists;
update the checks in the validate function (where entry and prefix are used) to
require non-empty lists by verifying isinstance(..., list) and len(...) > 0, and
append a clear error like "{prefix}: 'images' must be a non-empty list" or
"{prefix}: 'tags' must be a non-empty list" when the check fails.

Copy link
Copy Markdown
Member Author

@pfeerick pfeerick May 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

while not recommended, images list can be empty. tags should not be though.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pfeerick, thanks for the clarification! In that case, only tags needs the non-empty enforcement. Here's the revised fix:

-        if "images" in entry and not isinstance(entry["images"], list):
-            errors.append(f"{prefix}: 'images' must be a list")
+        if "images" in entry and not isinstance(entry["images"], list):
+            errors.append(f"{prefix}: 'images' must be a list")  # empty list is allowed

-        if "tags" in entry and not isinstance(entry["tags"], list):
-            errors.append(f"{prefix}: 'tags' must be a list")
+        if "tags" in entry:
+            if not isinstance(entry["tags"], list) or not entry["tags"]:
+                errors.append(f"{prefix}: 'tags' must be a non-empty list")

This keeps images as a type-only check (empty list passes) while requiring tags to be a non-empty list.

ʕ •ᴥ•ʔ 🐇✏️


✏️ Learnings added
Learnt from: pfeerick
Repo: EdgeTX/lua-scripts PR: 53
File: tools/validate_scripts.py:0-0
Timestamp: 2026-05-13T01:50:18.150Z
Learning: In `tools/validate_scripts.py` (EdgeTX/lua-scripts repo), the `images` field in `scripts.json` entries is allowed to be an empty list (while not recommended), so only a type check (`isinstance(entry["images"], list)`) is needed. The `tags` field, however, must be a non-empty list and should be validated with both `isinstance(..., list)` and a non-empty check (`not entry["tags"]`).

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.


name = entry.get("name")
if isinstance(name, str) and name.strip():
key = name.strip().lower()
if key in seen_names:
errors.append(
f"{prefix}: duplicate name '{name}'"
f" (also at entry {seen_names[key]})"
)
else:
seen_names[key] = i

return errors


def main() -> None:
parser = argparse.ArgumentParser(description="Validate scripts.json schema")
parser.add_argument(
"--scripts-json",
default="scripts.json",
type=Path,
help="Path to scripts.json (default: scripts.json)",
)
args = parser.parse_args()

data = load_and_parse(args.scripts_json)
errors = validate(data)

if errors:
print(f"Found {len(errors)} error(s) in {args.scripts_json}:")
for error in errors:
print(f" - {error}")
sys.exit(1)

print(f"{args.scripts_json} is valid ({len(data)} entries).")


if __name__ == "__main__":
main()
Loading