Skip to content

ci: exclude pyproject.toml from integration test triggers#92

Merged
Kamilbenkirane merged 1 commit intomainfrom
ci/exclude-pyproject-from-filters
Dec 19, 2025
Merged

ci: exclude pyproject.toml from integration test triggers#92
Kamilbenkirane merged 1 commit intomainfrom
ci/exclude-pyproject-from-filters

Conversation

@Kamilbenkirane
Copy link
Copy Markdown
Member

Version-only bumps no longer trigger unnecessary integration tests for unrelated packages.

@claude
Copy link
Copy Markdown

claude Bot commented Dec 19, 2025

Pull Request Review

Summary

This PR adds exclusion filters for pyproject.toml files to prevent version-only bumps from triggering integration tests for packages that haven't actually changed functionally.


✅ Positives

  1. Clear Intent: The PR effectively solves the stated problem - preventing unnecessary integration test runs when only version numbers change in pyproject.toml files.

  2. Consistent Application: The exclusion pattern is applied consistently across all capability packages (text-generation, image-generation, video-generation, speech-generation) and the core package.

  3. Low Risk: This is a minimal, focused change that only affects CI behavior, not application code.


🔍 Code Quality & Best Practices

Good:

  • The change is minimal and surgical
  • Uses standard glob negation syntax (!**/pyproject.toml)
  • Maintains consistency across all filters

Potential Concern:
The pattern !**/pyproject.toml will exclude ALL pyproject.toml files anywhere in the filtered paths, not just the top-level one. Based on the repository structure, each capability package has a single pyproject.toml at the root (e.g., packages/capabilities/text-generation/pyproject.toml), so this should be fine. However, if there were nested pyproject.toml files with actual code changes, those would also be excluded.

Recommendation: Consider using a more specific pattern like !pyproject.toml (without the **/ prefix) to only exclude the root-level pyproject.toml in each package directory. This would be more precise and self-documenting.


🐛 Potential Issues

  1. Missing Provider Packages: I notice the repository has several provider packages under packages/providers/ (anthropic, byteplus, google, elevenlabs, bfl, mistral, etc.) that also have pyproject.toml files. These providers aren't included in the filters at all. Should they be? If providers can be published independently, they might need similar filter logic.

  2. Edge Case - Dependency Changes: If a pyproject.toml file changes dependencies (not just version), this could affect package behavior and should trigger tests. The current implementation would skip tests in this case. Consider whether this trade-off is acceptable or if a more sophisticated check is needed (e.g., checking if only the version field changed).


⚡ Performance Considerations

Positive Impact: This change will reduce unnecessary CI runs, saving compute resources and reducing workflow execution time when doing version-only releases.

No negative performance impacts identified.


🔒 Security Concerns

No security issues identified. This is a CI configuration change that doesn't affect runtime code or introduce security vulnerabilities.


🧪 Test Coverage

Testing Recommendation: While this is a workflow change, you could verify the behavior by:

  1. Creating a test branch with only a version bump in a pyproject.toml
  2. Pushing a tag and confirming integration tests are skipped for that package
  3. Creating another test with both version bump AND code changes to confirm tests still run

The dorny/paths-filter action is well-tested, so the main risk is misconfigured patterns.


📝 Additional Suggestions

  1. Documentation: Consider adding a comment in the workflow file explaining why pyproject.toml is excluded, to help future maintainers understand the intent.

    filters: |
      text-generation:
        - 'packages/capabilities/text-generation/**'
        - '!**/pyproject.toml'  # Exclude version-only bumps
  2. Alternative Approach: If you want to be more precise, you could check if ONLY the version changed using a custom script, though that adds complexity.


🎯 Verdict

APPROVED

This is a sensible optimization with clear benefits. The implementation is correct for the stated goal. The only concerns are edge cases around dependency changes and potential inconsistency with provider packages, but these are minor and can be addressed in follow-up work if needed.

The risk/reward ratio is favorable - the change is simple, the benefits are clear, and the potential downsides are minimal and well-understood.

@Kamilbenkirane Kamilbenkirane merged commit 1461831 into main Dec 19, 2025
11 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