Skip to content

OE-01: Add CI workflow for fork - #3

Merged
MateoTTR merged 3 commits into
developfrom
feature/ci-workflow
Mar 28, 2026
Merged

OE-01: Add CI workflow for fork#3
MateoTTR merged 3 commits into
developfrom
feature/ci-workflow

Conversation

@MateoTTR

Copy link
Copy Markdown
Owner

Summary

  • Add .github/workflows/fork-ci.yml — a CI pipeline for the fork that runs on push to develop and PRs targeting develop
  • Checks: unit tests (with mock OPENAI_API_KEY), Black formatting, isort import sorting, mypy type checking
  • Uses ubuntu-latest runner with Python 3.10, pip caching for faster builds

Closes #1

Test plan

  • Verify workflow triggers on push to develop
  • Verify workflow triggers on PR targeting develop
  • Confirm unit tests pass with OPENAI_API_KEY=test
  • Confirm black --check passes
  • Confirm isort --check-only passes
  • Confirm mypy openevolve passes
  • Validate YAML syntax is correct

🤖 Generated with Claude Code

Closes #1

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

- name: Type check (mypy)
run: |
python -m mypy openevolve

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

CRITICAL — mypy step will permanently block CI on the existing codebase.

Running python -m mypy openevolve on the current codebase produces 232 errors in 19 files due to the strict settings in pyproject.toml (disallow_untyped_defs, disallow_incomplete_defs, warn_return_any). This means every PR will have a red CI from day one.

Two options:

Option A — Add --ignore-missing-imports and a baseline file (recommended for gradual adoption):

- name: Type check (mypy)
  run: |
    python -m mypy openevolve --ignore-missing-imports

Then also create a .mypy.ini or mypy.ini baseline with the current errors excluded, or relax the global pyproject.toml settings with ignore_missing_imports = true.

Option B — Set continue-on-error: true (quick fix, but mypy becomes non-blocking):

- name: Type check (mypy)
  continue-on-error: true
  run: |
    python -m mypy openevolve

Without one of these changes, this CI workflow will be red immediately after merge and cannot gate future PRs.

python -m unittest discover -s tests -p "test_*.py" -v

- name: Check formatting (black)
run: |

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

CRITICAL — Black check will fail immediately on the existing codebase.

Running python -m black --check openevolve tests reports 41 files would be reformatted on the current HEAD of develop. This will make the CI permanently red from the moment this workflow is merged.

The fix requires a pre-merge formatting pass:

python -m black openevolve examples tests scripts
git add -u && git commit -m "Apply black formatting"

Note: The upstream CLAUDE.md specifies formatting should include examples and scripts directories too (python -m black openevolve examples tests scripts). Consider widening the check scope here to match — though omitting them is fine as a minimum to reduce false positives from example files.

Comment thread .github/workflows/fork-ci.yml Outdated

- name: Check import sorting (isort)
run: |
python -m isort --check-only openevolve tests

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

CRITICAL — isort check will also fail on the existing codebase.

Running python -m isort --check-only openevolve tests currently reports errors in tests/integration/ files. These need to be fixed before CI is green.

Run to fix:

python -m isort openevolve tests
git add -u && git commit -m "Apply isort formatting"

Both this fix and the black fix above can be combined into one commit.

# Use ubuntu-latest for GitHub-hosted runners.
# To use a self-hosted runner, replace with: runs-on: self-hosted
runs-on: ubuntu-latest

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

SUGGESTION — No timeout-minutes on the job.

Without a job-level timeout, a hung test (e.g. a subprocess that never exits) will consume the full GitHub Actions default of 360 minutes and waste runner quota.

Add a timeout matching or slightly above the expected maximum run time:

jobs:
  lint-and-test:
    runs-on: ubuntu-latest
    timeout-minutes: 15

For reference, the upstream python-test.yml sets timeout-minutes: 30 on the integration job. 15 minutes is a reasonable cap for the unit test suite (currently ~65 seconds locally).

@MateoTTR MateoTTR left a comment

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Code Review — PR #3

Summary

The workflow structure, YAML syntax, trigger configuration, and individual step ordering are all correct. The issue is that the CI will be permanently red from day one because the existing develop branch does not yet satisfy any of the three quality checks: black, isort, and mypy all fail on the current codebase. These are blockers that must be resolved before merge.

Verdict: REQUEST CHANGES (posted as COMMENT — cannot request changes on own PR)
Upstream-ready: N/A (fork-internal, not upstream-able per issue spec)


Stats

  • Files reviewed: 1 (.github/workflows/fork-ci.yml)
  • Critical: 3 | Warnings: 0 | Suggestions: 1

CRITICAL issues (inline comments posted)

1. mypy step — 232 errors on current codebase (line 52)
python -m mypy openevolve fails with 232 errors in 19 files due to strict pyproject.toml settings (disallow_untyped_defs, warn_return_any). The step will block every PR from day one. Recommended fix: add --ignore-missing-imports as a minimum, and either relax pyproject.toml mypy settings or add a baseline to suppress pre-existing errors.

2. black check — 41 files would be reformatted (line 44)
python -m black --check openevolve tests fails against the current develop HEAD. A formatting pass must land on develop before or alongside this workflow file.

3. isort check — integration test files have sorting errors (line 49)
python -m isort --check-only openevolve tests fails on files in tests/integration/. Same pre-merge cleanup required.


Suggested fix ordering

  1. Run python -m black openevolve tests && python -m isort openevolve tests on develop, commit
  2. Decide on mypy strategy: either relax pyproject.toml settings (ignore_missing_imports = true, remove disallow_untyped_defs) or add continue-on-error: true as a temporary measure
  3. Re-push this PR branch — CI should be green before merge

SUGGESTION (inline comment posted)

4. Missing timeout-minutes on job (line 14)
Add timeout-minutes: 15 to prevent runaway jobs from consuming 360-minute default quota. Unit tests currently complete in ~65 seconds locally.


GOOD

  • YAML syntax is valid — confirmed with yaml.safe_load
  • Trigger configuration (push: develop, pull_request: develop) exactly matches the spec in .claude/CLAUDE.md Phase 1
  • Action versions upgraded to latest (v4/v5) compared to upstream's workflow
  • Cache key correctly uses pyproject.toml (not the non-existent requirements.txt that the upstream workflow references — this is an improvement)
  • OPENAI_API_KEY: test correctly scoped only to the test step — not leaked to lint/mypy steps
  • Test command python -m unittest discover -s tests -p "test_*.py" -v exactly matches .claude/CLAUDE.md dev commands — unit tests currently pass (370 tests, 0 failures)
  • Step ordering is logical: install -> test -> lint -> type-check

Automated review by Claude Code

MateoTTR and others added 2 commits March 28, 2026 13:12
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@MateoTTR
MateoTTR marked this pull request as ready for review March 28, 2026 12:43
@MateoTTR
MateoTTR merged commit a90fdaa into develop Mar 28, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant