Skip to content

ci: add CI workflow for automated testing on PRs#24

Merged
jack-arturo merged 4 commits into
mainfrom
ci/add-workflows
Dec 10, 2025
Merged

ci: add CI workflow for automated testing on PRs#24
jack-arturo merged 4 commits into
mainfrom
ci/add-workflows

Conversation

@jack-arturo
Copy link
Copy Markdown
Member

  • Runs tests on Python 3.10, 3.11, 3.12
  • Lint check with flake8 (syntax errors only block, style warnings allowed)
  • Coverage reporting to Codecov (optional)

- Runs tests on Python 3.10, 3.11, 3.12
- Lint check with flake8 (syntax errors only block, style warnings allowed)
- Coverage reporting to Codecov (optional)
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented Dec 10, 2025

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

📝 Walkthrough

Walkthrough

Adds a GitHub Actions CI workflow that runs tests and linting across Python 3.10, 3.11, and 3.12. The workflow includes flake8 linting, pytest test execution, and a coverage job that reports to codecov with dependency on test job completion.

Changes

Cohort / File(s) Summary
CI/CD Configuration
\.github/workflows/ci\.yml
New GitHub Actions workflow with matrix job across Python versions 3.10, 3.11, 3.12; includes checkout, Python setup with caching, dependency installation, flake8 linting (strict and relaxed modes), pytest test execution, and separate coverage job with codecov upload

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~5 minutes

  • Verify correct Python versions in matrix match project requirements
  • Confirm dependencies in requirements-dev.txt are sufficient for all workflow steps
  • Check that CODECOV_TOKEN is properly configured as a repository secret
  • Validate test path exclusion (tests/ --ignore=tests/benchmarks) matches actual test structure

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly describes the main change: adding a CI workflow for automated testing on pull requests, which matches the changeset that adds .github/workflows/ci.yml.
Description check ✅ Passed The description is directly related to the changeset, detailing the key features of the CI workflow: Python version matrix, lint checks with flake8, and coverage reporting.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai Bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

🧹 Nitpick comments (4)
.github/workflows/ci.yml (4)

31-34: Consolidate duplicate dependency installation steps.

The "Install dependencies" step is duplicated across the test and coverage jobs. Consider extracting this into a composite action or using a single source of truth to reduce maintenance overhead.

Example composite action approach (create .github/actions/install-deps/action.yml):

name: Install dependencies
description: Install Python dependencies from requirements-dev.txt
inputs:
  extra-packages:
    description: Additional packages to install
    required: false
    default: ''
runs:
  using: composite
  steps:
    - run: |
        python -m pip install --upgrade pip
        pip install -r requirements-dev.txt
        ${{ inputs.extra-packages }}
      shell: bash

Then reference it in both jobs:

- uses: ./.github/actions/install-deps
  with:
    extra-packages: pip install pytest-cov  # Only for coverage job

Also applies to: 61-65


48-76: Consider whether coverage should run on all Python versions.

Currently, coverage reporting runs only on Python 3.11. If there are version-specific bugs or differences in code paths across Python 3.10, 3.11, and 3.12, coverage metrics will only reflect the 3.11 path. Consider either:

  1. Running coverage on all matrix versions and aggregating results
  2. Documenting why 3.11 is the canonical coverage-reporting version

This approach would provide more comprehensive coverage data but increases CI time. Verify with the team whether this trade-off is acceptable.


36-41: Move flake8 configuration to an external config file.

The flake8 linting logic is embedded inline in the workflow. Best practice is to externalize this configuration to .flake8 or pyproject.toml for better maintainability and consistency with local development tooling.

Example .flake8 file:

[flake8]
max-line-length = 100
select = E9,F63,F7,F82
statistics = True
show-source = True

[flake8:local-plugin:warnings]
max-line-length = 100
exit-zero = True

Then simplify the workflow step:

- name: Lint with flake8
  run: flake8 app.py automem/

15-46: Add job timeout to prevent hanging builds.

The test job lacks a timeout specification. If a test hangs, the workflow will run until GitHub's 6-hour default limit. Set an explicit timeout for faster feedback.

Apply this diff:

  test:
    runs-on: ubuntu-latest
+   timeout-minutes: 30
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between d9bdf4f and ab9f03f.

📒 Files selected for processing (1)
  • .github/workflows/ci.yml (1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📓 Common learnings
Learnt from: CR
Repo: verygoodplugins/automem PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-09T02:15:31.644Z
Learning: Ensure CI passes and code is formatted/linted clean before merging pull requests
📚 Learning: 2025-12-09T02:15:31.644Z
Learnt from: CR
Repo: verygoodplugins/automem PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-12-09T02:15:31.644Z
Learning: Ensure CI passes and code is formatted/linted clean before merging pull requests

Applied to files:

  • .github/workflows/ci.yml
🔇 Additional comments (2)
.github/workflows/ci.yml (2)

71-75: Verify codecov token configuration and consider fail_ci_if_error setting.

Line 75 sets fail_ci_if_error: false, which allows the build to pass even if coverage upload fails. This is reasonable for optional reporting, but ensure:

  1. The CODECOV_TOKEN secret is configured in repository settings
  2. The team agrees that codecov upload failures should not block the build

If codecov reporting is critical, consider changing to fail_ci_if_error: true. If it's truly optional, keep as-is but document the decision.


8-12: Workflow structure and triggers are well-configured.

The trigger conditions (push to main + PR to main) align with the PR objective of ensuring CI passes before merging. Matrix strategy across Python 3.10, 3.11, 3.12 provides good compatibility testing coverage. Job dependency (coverage depends on test) prevents unnecessary codecov uploads on test failure.

- Black for formatting
- Isort for import sorting
- Flake8 for syntax errors
- Conventional commit message validation
- Detect-secrets for security
- Trailing whitespace and YAML checks
- Runs pre-commit checks (formatting, linting, secrets) as first step
- Tests only run after pre-commit passes
- Catches issues before tests even run
- Fix missing RECALL_EXPANSION_LIMIT import in app.py
- Fix pre-commit config (flake8 args, detect-secrets version)
- Auto-format all Python files with black (100 char line length)
- Sort imports with isort (black profile)
- Fix trailing whitespace and end-of-file issues
@jack-arturo jack-arturo merged commit 0e38d97 into main Dec 10, 2025
6 checks passed
@jack-arturo jack-arturo deleted the ci/add-workflows branch December 10, 2025 09:32
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