Skip to content

Configure celeste-text-generation for PyPI publication#21

Merged
Kamilbenkirane merged 1 commit intomainfrom
configure-text-generation-pypi
Nov 7, 2025
Merged

Configure celeste-text-generation for PyPI publication#21
Kamilbenkirane merged 1 commit intomainfrom
configure-text-generation-pypi

Conversation

@Kamilbenkirane
Copy link
Copy Markdown
Member

This PR configures the celeste-text-generation package for PyPI publication:

  • Renames package from text-generation to celeste-text-generation to avoid PyPI name conflict
  • Updates optional dependencies in root pyproject.toml to reference celeste-text-generation
  • Adds workspace source configuration so uv knows it's a workspace package
  • Updates publish workflow to build all workspace packages using --all-packages flag

When a tag is pushed, both celeste-ai and celeste-text-generation will be built and published to PyPI.

- Rename package from 'text-generation' to 'celeste-text-generation' to avoid PyPI conflict
- Update optional dependencies to reference celeste-text-generation
- Add workspace source configuration for celeste-text-generation
- Update publish workflow to build all workspace packages (--all-packages)
@Kamilbenkirane Kamilbenkirane merged commit 180091b into main Nov 7, 2025
11 checks passed
@claude
Copy link
Copy Markdown

claude Bot commented Nov 7, 2025

Pull Request Review: Configure celeste-text-generation for PyPI publication

Summary

This PR configures the workspace to properly publish both celeste-ai and celeste-text-generation packages to PyPI. The changes are well-structured and address a real PyPI naming conflict.

✅ Strengths

  1. Clear Problem & Solution: Renaming from text-generation to celeste-text-generation properly namespaces the package and avoids PyPI conflicts.

  2. Proper Workspace Configuration: The [tool.uv.sources] configuration correctly marks celeste-text-generation as a workspace package, which is essential for uv to resolve dependencies properly.

  3. Good Dependency Management: Optional dependencies in the root pyproject.toml allow users to install text-generation capabilities selectively with celeste-ai[text-generation] or celeste-ai[all].

🔍 Issues & Concerns

1. Critical: Version Synchronization Problem

Severity: High

Both packages share version 0.1.0, but the validate-release job (.github/workflows/publish.yml:12-37) only checks the root pyproject.toml version against the git tag. This means:

  • If you tag v0.1.0, it validates only celeste-ai version
  • celeste-text-generation could have a different version (e.g., 0.2.0) and still publish
  • No validation ensures workspace packages have synchronized versions

Recommendation:
Add a validation step to ensure all workspace package versions match:

- name: Validate workspace versions
  run: |
    ROOT_VERSION=$(python3 -c "import tomllib; print(tomllib.load(open('pyproject.toml', 'rb'))['project']['version'])")
    for pkg in packages/*/pyproject.toml; do
      PKG_VERSION=$(python3 -c "import tomllib; print(tomllib.load(open('$pkg', 'rb'))['project']['version'])")
      PKG_NAME=$(python3 -c "import tomllib; print(tomllib.load(open('$pkg', 'rb'))['project']['name'])")
      if [ "$PKG_VERSION" != "$ROOT_VERSION" ]; then
        echo "Version mismatch: $PKG_NAME has $PKG_VERSION but root has $ROOT_VERSION"
        exit 1
      fi
    done

2. Missing: Update Environment URLs

Severity: Medium

The publish workflow environments still only reference celeste-ai:

# publish.yml:85, 104
url: https://test.pypi.org/project/celeste-ai/
url: https://pypi.org/project/celeste-ai/

Recommendation:
Update environment descriptions to reflect both packages.

3. Potential Issue: Dependency Version Constraint

Severity: Low

The root pyproject.toml uses:

text-generation = ["celeste-text-generation>=0.1.0"]

This allows any version >=0.1.0, which could cause issues if someone installs celeste-ai==0.1.0 but gets celeste-text-generation==0.2.0 from PyPI.

Recommendation:
Use exact version matching for workspace packages:

text-generation = ["celeste-text-generation==0.1.0"]

Or use compatible release clause:

text-generation = ["celeste-text-generation~=0.1.0"]  # >=0.1.0, <0.2.0

4. Missing: Test Coverage

Severity: Medium

No tests verify:

  • Workspace package resolution works correctly
  • Optional dependencies can be installed successfully
  • Both packages build correctly with uv build --all-packages

Recommendation:
Add a CI test that validates the build produces both packages.

5. Documentation: Update Installation Instructions

Severity: Low

The README in packages/text-generation/README.md:32 correctly shows installation via the main package, but should also document standalone installation of celeste-text-generation.

🔒 Security Considerations

✅ No security concerns identified. The changes:

  • Do not modify authentication or credential handling
  • Use trusted PyPI publishing with OIDC (id-token: write)
  • Do not introduce new dependencies

⚡ Performance Considerations

✅ No performance impact. Changes are configuration-only.

🧪 Test Coverage

⚠️ Gap: No automated tests for workspace build and publishing configuration.

Suggested tests:

  1. Test that uv build --all-packages produces both packages
  2. Test that optional dependencies resolve correctly
  3. Test that workspace sources are properly configured

📋 Code Quality & Best Practices

Good:

  • Follows uv workspace conventions
  • Clean, minimal changes
  • Proper TOML formatting
  • Descriptive PR description

⚠️ Could improve:

  • Add version synchronization validation
  • Add build verification tests

🎯 Recommendation

Conditional Approval - The PR is functionally correct but has important gaps:

  1. Must fix before merge:

    • Add version synchronization validation to prevent mismatched versions
  2. Should fix before merge:

    • Add build verification test
    • Update dependency constraints to use exact version or compatible release
  3. Nice to have:

    • Update environment URLs
    • Add standalone installation docs

Additional Notes

The change from uv build to uv build --all-packages is correct and necessary for multi-package workspaces. However, this is a significant behavior change that warrants additional validation in CI.

Consider whether you want both packages to always be released together (current approach) or if they should have independent release cycles. If independent, you will need more sophisticated workflow logic.

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