ci: Détection hebdomadaire des changements d'API micropython-steami-lib#121
Conversation
- scripts/extract-api.py: extracts public API signatures using ast - scripts/api-snapshot.json: initial snapshot (12 drivers) - scripts/check-api-drift.py: compares current API against snapshot - .github/workflows/api-drift.yml: weekly check, creates issue on drift Closes #112
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 87d0faded4
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| else | ||
| echo "changes=false" >> "$GITHUB_OUTPUT" |
There was a problem hiding this comment.
The workflow currently treats every non-1 exit code as changes=false, so an extractor/checker failure (for example any future sys.exit(2) error path documented in check-api-drift.py, or a command/runtime failure) is silently reported as “no API changes.” This can hide real monitoring outages and make scheduled runs look healthy while drift detection is broken; unexpected exit codes should fail the step instead of falling through to the changes=false branch.
Useful? React with 👍 / 👎.
| for arg in item.args.args: | ||
| if arg.arg == "self": | ||
| continue | ||
| args.append(arg.arg) |
There was a problem hiding this comment.
The extractor only records item.args.args, which excludes positional-only, keyword-only, *args, and **kwargs parameters from method signatures. As a result, API changes that modify those parameter kinds will not appear in the snapshot and check-api-drift.py can miss real signature drift, undermining the purpose of the weekly compatibility check.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Pull request overview
Ce PR ajoute une détection hebdomadaire des dérives d’API de micropython-steami-lib afin d’ouvrir/mettre à jour automatiquement une issue quand les signatures publiques des drivers changent (pour déclencher la mise à jour de la documentation).
Changes:
- Ajout d’un extracteur AST pour générer un snapshot JSON des classes/méthodes publiques (
scripts/extract-api.py) et d’un comparateur générant un rapport Markdown (scripts/check-api-drift.py). - Ajout d’un snapshot initial des signatures publiques (
scripts/api-snapshot.json). - Ajout d’un workflow GitHub Actions planifié + manuel qui clone
micropython-steami-lib, compare l’API et ouvre/alimente une issue (.github/workflows/api-drift.yml). - Mise à jour de
.gitignorepour les artefacts Python.
Reviewed changes
Copilot reviewed 4 out of 5 changed files in this pull request and generated 13 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/extract-api.py | Extraction AST des classes/méthodes et paramètres pour construire un snapshot d’API. |
| scripts/check-api-drift.py | Comparaison snapshot vs API courante + génération d’un rapport Markdown et codes de sortie. |
| scripts/api-snapshot.json | Snapshot initial de l’API (drivers/classes/méthodes/params). |
| .github/workflows/api-drift.yml | Exécution hebdomadaire + manuelle, création/commentaire d’issue si dérive détectée. |
| .gitignore | Ignore __pycache__/ et *.pyc. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| spec = importlib.util.spec_from_file_location( | ||
| "extract_api", script_dir / "extract-api.py" | ||
| ) | ||
| mod = importlib.util.module_from_spec(spec) | ||
| spec.loader.exec_module(mod) |
There was a problem hiding this comment.
load_extract_module() assumes spec and spec.loader are always non-null. If the file can’t be found or the loader can’t be created, this will raise an exception with an unclear stack trace (and may be misinterpreted by the workflow). Add explicit checks and emit a clear error message, exiting with code 2.
| spec = importlib.util.spec_from_file_location( | |
| "extract_api", script_dir / "extract-api.py" | |
| ) | |
| mod = importlib.util.module_from_spec(spec) | |
| spec.loader.exec_module(mod) | |
| extract_path = script_dir / "extract-api.py" | |
| spec = importlib.util.spec_from_file_location("extract_api", extract_path) | |
| if spec is None or spec.loader is None: | |
| print( | |
| f"Error: could not create a module spec for {extract_path}", | |
| file=sys.stderr, | |
| ) | |
| sys.exit(2) | |
| try: | |
| mod = importlib.util.module_from_spec(spec) | |
| spec.loader.exec_module(mod) | |
| except FileNotFoundError: | |
| print( | |
| f"Error: extract-api.py not found at {extract_path}", | |
| file=sys.stderr, | |
| ) | |
| sys.exit(2) | |
| except Exception as exc: | |
| print( | |
| f"Error: failed to load extract-api module from {extract_path}: {exc}", | |
| file=sys.stderr, | |
| ) | |
| sys.exit(2) |
| extract_api = load_extract_module() | ||
| current = extract_api.scan_all_drivers(lib_dir) | ||
| stored = load_snapshot(snapshot_file) |
There was a problem hiding this comment.
Main flow doesn’t catch errors from API extraction / snapshot loading, so unexpected failures can exit with code 1 and be treated as “drift detected” by the workflow. Wrap this section in try/except and ensure genuine errors exit with code 2 (and print the error to stderr).
| extract_api = load_extract_module() | |
| current = extract_api.scan_all_drivers(lib_dir) | |
| stored = load_snapshot(snapshot_file) | |
| try: | |
| extract_api = load_extract_module() | |
| current = extract_api.scan_all_drivers(lib_dir) | |
| stored = load_snapshot(snapshot_file) | |
| except Exception as exc: | |
| print(f"Error while extracting API or loading snapshot: {exc}", file=sys.stderr) | |
| sys.exit(2) |
| if driver not in stored: | ||
| changes.append(f"### Nouveau driver : `{driver}`") | ||
| for cls, methods in cur_classes.items(): | ||
| changes.append(f"- Classe `{cls}` avec {len(methods)} methodes") |
There was a problem hiding this comment.
Typo in French output: “methodes” should be “méthodes” (accent). This string is user-visible in the generated Markdown report/issue.
| changes.append(f"- Classe `{cls}` avec {len(methods)} methodes") | |
| changes.append(f"- Classe `{cls}` avec {len(methods)} méthodes") |
| REPORT=$(python scripts/check-api-drift.py micropython-steami-lib/lib scripts/api-snapshot.json) | ||
| EXIT_CODE=$? | ||
| set -e | ||
|
|
||
| echo "exit_code=$EXIT_CODE" >> "$GITHUB_OUTPUT" | ||
|
|
||
| if [ "$EXIT_CODE" -eq 1 ]; then | ||
| # Write multiline report to file for the next step | ||
| echo "$REPORT" > /tmp/api-drift-report.md |
There was a problem hiding this comment.
Capturing the Markdown report via command substitution (REPORT=$(python ...)) can drop trailing newlines and may behave poorly for large multi-line outputs. Prefer writing the script output directly to a file (and/or using printf), then consume that file in the next step.
| REPORT=$(python scripts/check-api-drift.py micropython-steami-lib/lib scripts/api-snapshot.json) | |
| EXIT_CODE=$? | |
| set -e | |
| echo "exit_code=$EXIT_CODE" >> "$GITHUB_OUTPUT" | |
| if [ "$EXIT_CODE" -eq 1 ]; then | |
| # Write multiline report to file for the next step | |
| echo "$REPORT" > /tmp/api-drift-report.md | |
| python scripts/check-api-drift.py micropython-steami-lib/lib scripts/api-snapshot.json > /tmp/api-drift-report.md | |
| EXIT_CODE=$? | |
| set -e | |
| echo "exit_code=$EXIT_CODE" >> "$GITHUB_OUTPUT" | |
| if [ "$EXIT_CODE" -eq 1 ]; then | |
| # Report already written to /tmp/api-drift-report.md for the next step |
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| issue_number: existing.data[0].number, | ||
| body: `Mise a jour ${new Date().toISOString().split('T')[0]} :\n\n${report}`, |
There was a problem hiding this comment.
Typo in issue/comment body: “Mise a jour” should be “Mise à jour” (accent).
| body: `Mise a jour ${new Date().toISOString().split('T')[0]} :\n\n${report}`, | |
| body: `Mise à jour ${new Date().toISOString().split('T')[0]} :\n\n${report}`, |
| args = [] | ||
| for arg in item.args.args: | ||
| if arg.arg == "self": | ||
| continue | ||
| args.append(arg.arg) |
There was a problem hiding this comment.
Argument extraction only looks at item.args.args (positional-or-keyword). It ignores positional-only args, keyword-only args, *args, and **kwargs, so parameter changes involving those won’t be detected. Consider including posonlyargs, kwonlyargs, and representing varargs/kwargs (e.g., *name, **name) in the stored signature.
| args = [] | |
| for arg in item.args.args: | |
| if arg.arg == "self": | |
| continue | |
| args.append(arg.arg) | |
| args = [] | |
| # Positional-only arguments (Python 3.8+) | |
| for arg in getattr(item.args, "posonlyargs", []): | |
| if arg.arg == "self": | |
| continue | |
| args.append(arg.arg) | |
| # Positional-or-keyword arguments | |
| for arg in item.args.args: | |
| if arg.arg == "self": | |
| continue | |
| args.append(arg.arg) | |
| # *args (vararg) | |
| if item.args.vararg is not None: | |
| if item.args.vararg.arg != "self": | |
| args.append("*" + item.args.vararg.arg) | |
| # Keyword-only arguments | |
| for arg in item.args.kwonlyargs: | |
| if arg.arg == "self": | |
| continue | |
| args.append(arg.arg) | |
| # **kwargs (kwarg) | |
| if item.args.kwarg is not None: | |
| if item.args.kwarg.arg != "self": | |
| args.append("**" + item.args.kwarg.arg) |
| print("No API changes detected.") | ||
| sys.exit(0) | ||
|
|
||
| report = "## Changements d'API detectes\n\n" + "\n".join(changes) |
There was a problem hiding this comment.
Typo in French output: “detectes” should be “détectés” (accents). This is the report title shown in the created issue.
| report = "## Changements d'API detectes\n\n" + "\n".join(changes) | |
| report = "## Changements d'API détectés\n\n" + "\n".join(changes) |
| return json.loads(Path(snapshot_path).read_text(encoding="utf-8")) | ||
|
|
||
|
|
There was a problem hiding this comment.
load_snapshot() doesn’t handle missing/invalid files (OSError) or invalid JSON (JSONDecodeError). Add error handling so these cases produce a controlled exit code 2 with a clear stderr message instead of a stack trace.
| return json.loads(Path(snapshot_path).read_text(encoding="utf-8")) | |
| try: | |
| content = Path(snapshot_path).read_text(encoding="utf-8") | |
| except OSError as e: | |
| print(f"Error reading snapshot file '{snapshot_path}': {e}", file=sys.stderr) | |
| sys.exit(2) | |
| try: | |
| return json.loads(content) | |
| except json.JSONDecodeError as e: | |
| print( | |
| f"Error parsing JSON from snapshot file '{snapshot_path}': {e}", | |
| file=sys.stderr, | |
| ) | |
| sys.exit(2) |
| body: `Mise a jour ${new Date().toISOString().split('T')[0]} :\n\n${report}`, | ||
| }); | ||
| } else { | ||
| // Create new issue | ||
| await github.rest.issues.create({ | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| title: 'docs: Changements API detectes dans micropython-steami-lib', | ||
| body: `${report}\n\n---\n*Detecte automatiquement par le workflow api-drift.*`, |
There was a problem hiding this comment.
Typo in issue body footer: “Detecte” should be “Détecté” (accent).
| body: `Mise a jour ${new Date().toISOString().split('T')[0]} :\n\n${report}`, | |
| }); | |
| } else { | |
| // Create new issue | |
| await github.rest.issues.create({ | |
| owner: context.repo.owner, | |
| repo: context.repo.repo, | |
| title: 'docs: Changements API detectes dans micropython-steami-lib', | |
| body: `${report}\n\n---\n*Detecte automatiquement par le workflow api-drift.*`, | |
| body: `Mise à jour ${new Date().toISOString().split('T')[0]} :\n\n${report}`, | |
| }); | |
| } else { | |
| // Create new issue | |
| await github.rest.issues.create({ | |
| owner: context.repo.owner, | |
| repo: context.repo.repo, | |
| title: 'docs: Changements API détectés dans micropython-steami-lib', | |
| body: `${report}\n\n---\n*Détecté automatiquement par le workflow api-drift.*`, |
| if [ "$EXIT_CODE" -eq 1 ]; then | ||
| # Write multiline report to file for the next step | ||
| echo "$REPORT" > /tmp/api-drift-report.md | ||
| echo "changes=true" >> "$GITHUB_OUTPUT" | ||
| else | ||
| echo "changes=false" >> "$GITHUB_OUTPUT" |
There was a problem hiding this comment.
The else branch currently treats any non-1 exit code as “no changes”. That will hide real errors (e.g., exit code 2 per script contract) and make the scheduled workflow look green while not checking anything. Handle 0/1 explicitly, and fail the job on any other exit code.
| if [ "$EXIT_CODE" -eq 1 ]; then | |
| # Write multiline report to file for the next step | |
| echo "$REPORT" > /tmp/api-drift-report.md | |
| echo "changes=true" >> "$GITHUB_OUTPUT" | |
| else | |
| echo "changes=false" >> "$GITHUB_OUTPUT" | |
| if [ "$EXIT_CODE" -eq 0 ]; then | |
| echo "changes=false" >> "$GITHUB_OUTPUT" | |
| elif [ "$EXIT_CODE" -eq 1 ]; then | |
| # Write multiline report to file for the next step | |
| echo "$REPORT" > /tmp/api-drift-report.md | |
| echo "changes=true" >> "$GITHUB_OUTPUT" | |
| else | |
| echo "API drift check failed with exit code $EXIT_CODE" >&2 | |
| exit "$EXIT_CODE" |
- Include __init__ in public API signatures - Capture all parameter kinds (posonlyargs, kwonlyargs, *args, **kwargs) - Add error handling in load_extract_module and load_snapshot - Catch OSError in extract_classes - Fix exit code handling in workflow (fail on code != 0/1) - Write report directly to file instead of command substitution - Fix French accents in output (méthodes, détectés, etc.) - Create api-drift label in repo
|
Tous les commentaires traités (d5cabe1) :
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d5cabe105e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| for arg in getattr(item.args, "posonlyargs", []): | ||
| if arg.arg == "self": | ||
| continue | ||
| args.append(arg.arg) |
There was a problem hiding this comment.
extract_methods flattens positional-only parameters into the same representation as regular positional-or-keyword parameters (and does the same for keyword-only parameters later), so kind-only signature changes are invisible to drift detection. For example, def f(self, a, *, b) and def f(self, a, b) both serialize as ['a', 'b'], which means check-api-drift.py will miss real API incompatibilities when parameter kinds change.
Useful? React with 👍 / 👎.
| const existing = await github.rest.issues.listForRepo({ | ||
| owner: context.repo.owner, | ||
| repo: context.repo.repo, | ||
| state: 'open', | ||
| labels: 'api-drift', | ||
| per_page: 1, |
There was a problem hiding this comment.
The workflow uses github.rest.issues.listForRepo with labels: 'api-drift' and immediately reuses the first result, but this endpoint can return pull requests as well as issues. If an open PR is labeled api-drift, existing.data[0] can point to that PR, so the job comments on the PR instead of the tracking issue and may never create/update the intended drift issue.
Useful? React with 👍 / 👎.
Summary
scripts/extract-api.py: extrait les signatures publiques des drivers avec le moduleastscripts/api-snapshot.json: snapshot initial (12 drivers, toutes les méthodes publiques avec paramètres)scripts/check-api-drift.py: compare l'API actuelle avec le snapshot et génère un rapport Markdown.github/workflows/api-drift.yml: exécution hebdomadaire (lundi 8h UTC) + déclenchement manuel. Crée une issue avec le labelapi-driftsi des changements sont détectés.Fonctionnement
Le workflow clone micropython-steami-lib, extrait l'API actuelle et la compare au snapshot. Il détecte :
Si une issue
api-driftest déjà ouverte, un commentaire est ajouté plutôt que de créer un doublon.Closes #112
Test plan
python scripts/check-api-drift.py micropython-steami-lib/lib scripts/api-snapshot.jsonretourne "No API changes detected"workflow_dispatchapi-driftest bien créé