Skip to content

add new composite actions, add a stale stubs check to type check workflow, refactor shared logic into composite actions #162

Merged
speediedan merged 4 commits intomainfrom
add_new_composite_action_and_stale_stubs
Aug 30, 2025
Merged

add new composite actions, add a stale stubs check to type check workflow, refactor shared logic into composite actions #162
speediedan merged 4 commits intomainfrom
add_new_composite_action_and_stale_stubs

Conversation

@speediedan
Copy link
Copy Markdown
Owner

add new composite actions, add a stale stubs check to type check workflow, refactor shared logic into composite actions

@github-actions github-actions bot added area: ci Continuous integration area: scripts Shell scripts and automation config Configuration file changes labels Aug 29, 2025
@github-actions
Copy link
Copy Markdown

github-actions bot commented Aug 29, 2025

regen-ci-req-check detected changes to pinned CI requirements and uploaded a patch artifact named 'regen-pins-diff'.

Top changes:

  1. coverage: ==7.10.5 → ==7.10.6
  2. jupytext: ==1.17.2 → ==1.17.3
  3. pytest-rerunfailures: ==15.1 → ==16.0
  4. sae-lens: ==6.7.0 → ==6.9.0
  5. wandb: ==0.21.1 → ==0.21.2

Please review the artifact and CI results. This workflow is report-only and will not open a PR; the scheduled regen workflow will open PRs automatically.

… type stub generation to include formatting to ensure semantic diffs
@speediedan speediedan marked this pull request as ready for review August 30, 2025 00:52
Copilot AI review requested due to automatic review settings August 30, 2025 00:52
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR refactors CI workflows by extracting shared dependency installation logic into reusable composite actions and adds a stale stubs check to the type check workflow. The main goals are to improve maintainability by reducing code duplication and ensure type stubs remain synchronized with the codebase.

Key changes:

  • Creates composite actions for installing CI dependencies and checking file differences
  • Adds automated stale stub detection to prevent type checking against outdated stubs
  • Updates the stub generation script to include automatic formatting

Reviewed Changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
.github/actions/install-ci-dependencies/action.yml New composite action that centralizes CI dependency installation logic
.github/actions/check-file-diffs/action.yml New composite action for comparing files and generating patches
.github/workflows/type-check.yml Renamed and enhanced to check for stale stubs before type checking
.github/workflows/ci_test-full.yml Refactored to use the new install-ci-dependencies composite action
.github/workflows/copilot-setup-steps.yml Updated to use composite actions and added caching
.github/actions/regen-ci-reqs/action.yml Refactored to use the new check-file-diffs composite action
scripts/generate_op_stubs.py Enhanced to automatically format generated stubs
src/interpretune/__init__.pyi Updated type stubs with new imports

env:
APPLY_POST_UPGRADES: ${{ inputs.apply_post_upgrades }}
run: |
if ([ "${APPLY_POST_UPGRADES}" = "1" ] || [ "${APPLY_POST_UPGRADES}" = "true" ]) && [ -s requirements/post_upgrades.txt ]; then
Copy link

Copilot AI Aug 30, 2025

Choose a reason for hiding this comment

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

The condition logic is unnecessarily complex with nested brackets. Consider simplifying to if [[ "${APPLY_POST_UPGRADES}" =~ ^(1|true)$ ]] && [[ -s requirements/post_upgrades.txt ]]; then for better readability.

Suggested change
if ([ "${APPLY_POST_UPGRADES}" = "1" ] || [ "${APPLY_POST_UPGRADES}" = "true" ]) && [ -s requirements/post_upgrades.txt ]; then
if [[ "${APPLY_POST_UPGRADES}" =~ ^(1|true)$ ]] && [[ -s requirements/post_upgrades.txt ]]; then

Copilot uses AI. Check for mistakes.
Comment on lines +311 to +319

# Ensure pre-commit hooks are installed
# We run install every time since it's idempotent and fast if already installed
install_result = subprocess.run(["pre-commit", "install"], capture_output=True, text=True, cwd=project_root)

if install_result.returncode != 0:
print(f"Warning: Failed to install pre-commit hooks: {install_result.stderr}")
print("Skipping formatting step")
return
Copy link

Copilot AI Aug 30, 2025

Choose a reason for hiding this comment

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

Running pre-commit install on every stub generation is unnecessary overhead. Consider checking if hooks are already installed before running the install command, or document that this overhead is acceptable since it's idempotent.

Suggested change
# Ensure pre-commit hooks are installed
# We run install every time since it's idempotent and fast if already installed
install_result = subprocess.run(["pre-commit", "install"], capture_output=True, text=True, cwd=project_root)
if install_result.returncode != 0:
print(f"Warning: Failed to install pre-commit hooks: {install_result.stderr}")
print("Skipping formatting step")
return
import os
# Ensure pre-commit hooks are installed only if not already present
hook_path = project_root / ".git" / "hooks" / "pre-commit"
if not hook_path.exists():
install_result = subprocess.run(["pre-commit", "install"], capture_output=True, text=True, cwd=project_root)
if install_result.returncode != 0:
print(f"Warning: Failed to install pre-commit hooks: {install_result.stderr}")
print("Skipping formatting step")
return

Copilot uses AI. Check for mistakes.
echo "No platform-dependent packages to install."
fi
# Make a backup of current stubs
cp src/interpretune/__init__.pyi src/interpretune/__init__.pyi.backup
Copy link

Copilot AI Aug 30, 2025

Choose a reason for hiding this comment

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

The backup file creation appears unused since the workflow doesn't restore from it on failure. Either remove this line or add restoration logic to maintain the original state when the check fails.

Suggested change
cp src/interpretune/__init__.pyi src/interpretune/__init__.pyi.backup

Copilot uses AI. Check for mistakes.
@speediedan speediedan merged commit 6c38a1d into main Aug 30, 2025
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: ci Continuous integration area: scripts Shell scripts and automation config Configuration file changes

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants