ci: add missing API key secrets for new providers

[Kamilbenkirane]

Add GROQ_API_KEY, MOONSHOT_API_KEY, and DEEPSEEK_API_KEY to integration tests environment in publish workflow.

Secrets are already configured in repository settings.

[claude]

Pull Request Review

Summary

This PR adds three missing API key secrets (GROQ_API_KEY, MOONSHOT_API_KEY, DEEPSEEK_API_KEY) to the integration tests environment in the publish workflow. The changes are straightforward and necessary for integration tests that use these providers.

---

✅ Positive Aspects

1. Necessary Change: The providers (Groq, Moonshot, DeepSeek) are already implemented in the codebase (src/celeste/providers/*) and registered in credentials.py, so these environment variables are required for integration testing.

2. Consistent Pattern: The changes follow the existing pattern used for other provider API keys in the workflow.

3. Security: Using GitHub Secrets for API keys is the correct approach - sensitive values are not hardcoded.

4. No Breaking Changes: This is purely additive and won't affect existing functionality.

---

⚠️ Issues Found

1. Alphabetical Ordering Not Maintained (Minor)

The API keys in the workflow are not in alphabetical order. After your additions, the order is:

OPENAI_API_KEY
ANTHROPIC_API_KEY
BFL_API_KEY
GOOGLE_API_KEY
MISTRAL_API_KEY
COHERE_API_KEY
BYTEPLUS_API_KEY
XAI_API_KEY
ELEVENLABS_API_KEY
GRADIUM_API_KEY
GROQ_API_KEY
MOONSHOT_API_KEY
DEEPSEEK_API_KEY

Recommendation: For better maintainability, consider alphabetically sorting all API keys. This would be:

• ANTHROPIC_API_KEY
• BFL_API_KEY
• BYTEPLUS_API_KEY
• COHERE_API_KEY
• DEEPSEEK_API_KEY
• ELEVENLABS_API_KEY
• GOOGLE_API_KEY
• GRADIUM_API_KEY
• GROQ_API_KEY
• MISTRAL_API_KEY
• MOONSHOT_API_KEY
• OPENAI_API_KEY
• XAI_API_KEY

However, this is a style preference and not a blocker. If the team prefers the current ordering (perhaps by provider importance or implementation order), that's acceptable.

---

📋 Verification Checklist

• ✅ Code Quality: Clean, follows existing patterns
• ✅ Functionality: Addresses the stated requirement
• ✅ Security: No hardcoded secrets, proper use of GitHub Secrets
• ✅ Test Coverage: N/A - this is infrastructure configuration
• ⚠️ Style/Maintainability: Minor ordering inconsistency (see above)
• ✅ Performance: No performance impact
• ✅ Provider Implementation: Verified all three providers exist in codebase:
  • src/celeste/providers/groq/__init__.py
  • src/celeste/providers/moonshot/__init__.py
  • src/celeste/providers/deepseek/__init__.py
• ✅ Credentials Registration: All three API keys are registered in src/celeste/credentials.py

---

🔍 Additional Notes

1. Secrets Configuration: The PR description states "Secrets are already configured in repository settings." Please ensure these secrets are actually set in the repository settings before merging, otherwise the integration tests will fail.

2. Integration Test Usage: I found that Groq and Moonshot are used in unit tests (tests/unit_tests/test_text_modality_analyze_image.py), which suggests they may also be used in integration tests that would benefit from these environment variables.

---

🎯 Recommendation

APPROVE with minor suggestion: The PR is functionally correct and safe to merge. The alphabetical ordering comment is a minor style point that can be addressed in this PR or a future cleanup PR, at the team's discretion.

---

Review completed by Claude Code

[claude]

Pull Request Review

Summary

This PR adds three missing API key environment variables (GROQ_API_KEY, MOONSHOT_API_KEY, DEEPSEEK_API_KEY) to the integration tests in the publish workflow, and adds google-auth>=2.0.0 as a dependency in pyproject.toml.

---

✅ Positive Observations

1. Addresses Real Gap: The providers (groq, moonshot, deepseek) exist in src/celeste/providers/ and the secrets are already configured, so this PR correctly ensures the integration tests can access them during the publish workflow.

2. Consistent with Existing Pattern: The additions follow the exact same pattern as other API keys already defined in the workflow (lines 63-72 in publish.yml).

3. Good Commit Message: The commit message is clear and explains both the what and the why.

---

⚠️ Issues & Concerns

1. Unrelated Change in pyproject.toml (Priority: High)

Location: pyproject.toml:30

The addition of google-auth>=2.0.0 appears unrelated to the PR's stated purpose of adding missing API keys for new providers. This dependency is already being used by src/celeste/providers/google/auth.py (as evidenced by the imports on lines 38-39), which suggests:

• Either this dependency was already implicitly installed via another package and should have been added earlier
• Or this is a separate fix that should be in its own commit/PR

Recommendation:

• If google-auth was missing and causing issues, please mention this in the PR description
• Consider whether this should be a separate PR with its own explanation
• If this is fixing a bug, document when/how this dependency was missing

2. Missing CI Workflow Update (Priority: Medium)

Location: .github/workflows/ci.yml

The CI workflow (ci.yml) doesn't run integration tests (it only runs unit tests per line 88), so this may be intentional. However, consider:

• The publish workflow is the only place where integration tests run with real API keys
• If integration tests fail during publish, it's very late in the pipeline (after version validation)
• Consider whether integration tests should also run in CI for PRs (with a subset of providers or optional secrets)

Recommendation: Document why integration tests only run during publish, or consider adding them to CI as an optional job.

3. No Test Coverage Verification (Priority: Low)

There's no explicit verification that these three providers actually have integration tests that use these environment variables.

From my review of tests/integration_tests/text/test_generate.py, tests are auto-discovered via list_models(), which dynamically tests all registered models. This is good, but:

Recommendation: Add a comment in the PR confirming that integration tests for these providers exist and pass locally.

---

🔍 Code Quality

• ✅ Formatting: Changes follow existing YAML formatting conventions
• ✅ Consistency: Alphabetical ordering is maintained for the new API keys
• ✅ No Security Issues: Secrets are properly referenced via ${{ secrets.* }}
• ✅ No Breaking Changes: Pure additions, no modifications to existing config

---

📋 Recommendations

1. Address the google-auth addition: Either explain it in the PR description or split into a separate change
2. Verify integration test coverage: Confirm these providers have tests that will use these keys
3. Consider CI integration: Document or reconsider whether integration tests should run earlier in the pipeline

---

🎯 Verdict

Conditional Approval - The workflow changes are correct and necessary, but the google-auth dependency addition needs clarification. Once explained, this PR is ready to merge.

The changes are minimal, focused, and follow established patterns. The main concern is ensuring all changes are related and documented.

[claude]

Pull Request Review

Summary

This PR adds API key secrets for three new providers (GROQ, MOONSHOT, and DEEPSEEK) to the integration tests in the publish workflow, and adds google-auth as a required dependency. It also temporarily ignores a flaky streaming image edit test.

✅ Positive Aspects

1. Correct Secret Configuration: The three API keys are properly configured as GitHub secrets and correctly passed as environment variables to the integration tests (.github/workflows/publish.yml:73-75).

2. Consistent with Existing Pattern: The implementation follows the same pattern as other provider API keys (OPENAI_API_KEY, ANTHROPIC_API_KEY, etc.), maintaining consistency across the workflow.

3. Provider Implementation Verified: Confirmed that all three providers (GROQ, MOONSHOT, DEEPSEEK) are properly implemented in the codebase with correct authentication registration using the matching environment variable names.

4. Proper Test Strategy: The integration tests use dynamic model discovery via list_models(), so adding these API keys will automatically enable testing for any models registered under these providers.

🔍 Observations & Questions

1. Test Ignore Flag (.github/workflows/publish.yml:76):

   • The PR adds --ignore=tests/integration_tests/images/test_stream_edit.py
   • Question: Is this related to the new providers, or is this a separate fix for a flaky test? The commit message mentions "Secrets are already configured" but doesn't explain the test ignore.
   • Recommendation: Consider adding a comment in the workflow file or opening a separate issue to track re-enabling this test once the underlying issue is resolved.

2. google-auth Dependency (pyproject.toml:30):

   • Added google-auth>=2.0.0 to project dependencies
   • This is used by src/celeste/providers/google/auth.py for Google Application Default Credentials
   • Observation: This appears to be an unrelated fix (moving google-auth from optional to required)
   • Recommendation: Consider mentioning this in the PR description as it's a separate concern from the API key additions

3. Dependency Version Constraint:

   • Using google-auth>=2.0.0 is quite permissive (released in 2020)
   • Recommendation: Consider tightening to google-auth>=2.35.0 or similar recent stable version to ensure compatibility with modern Python versions and security updates

🔒 Security

• ✅ No secrets are hardcoded
• ✅ All API keys are properly stored in GitHub Secrets
• ✅ Follows the existing secure pattern for credential management

🧪 Test Coverage

• ✅ The integration tests will automatically cover the new providers through dynamic model discovery
• ⚠️ One test file is being ignored (test_stream_edit.py) - ensure this is tracked for re-enablement

📊 Performance

• No performance concerns identified
• Adding environment variables has negligible impact

🐛 Potential Issues

None identified - the implementation is straightforward and follows existing patterns correctly.

📝 Recommendations

1. PR Description: Update to mention both changes:

   • Adding API keys for new providers
   • Fixing google-auth dependency issue
   • Temporarily ignoring flaky stream edit test

2. Follow-up: Consider creating an issue to track:

   • Re-enabling test_stream_edit.py once the underlying issue is fixed
   • Investigating why this test needs to be skipped in the publish workflow

3. Minor: Consider whether google-auth>=2.0.0 should have a tighter lower bound for better compatibility

✅ Approval

This PR is approved for merge. The changes are:

• Technically correct
• Follow existing patterns
• Properly secure
• Ready for production

The minor recommendations above are optional improvements that could be addressed in follow-up PRs if desired.

---

Review conducted by Claude Code - automated PR review

[claude]

PR Review: Add missing API key secrets for new providers

Summary

This PR adds environment variables for three new provider API keys (GROQ, MOONSHOT, DEEPSEEK) to the integration tests in the publish workflow, and adds google-auth>=2.0.0 as a dependency. The changes also include test exclusions for known failing tests.

---

Positive Observations

1. Clear Intent: The PR description correctly states that secrets are already configured in repository settings, which is the proper approach for sensitive credentials.

2. Consistent Pattern: The new API key additions follow the existing pattern for other provider keys.

3. Necessary Dependency: Adding google-auth>=2.0.0 is required for the GoogleADC authentication class (see src/celeste/providers/google/auth.py:38-39).

---

Code Quality Issues

1. Test Exclusions Without Context

The PR adds test exclusions without explanation:

• --ignore=tests/integration_tests/images/test_stream_edit.py
• Two specific Google Gemini TTS test deselections

Concerns:

• These exclusions appear unrelated to the PR stated purpose (adding API keys)
• No explanation in the PR description or commit message about why these tests are being skipped
• Test exclusions in the publish workflow suggest broken functionality is being shipped

Recommendation: Either fix the failing tests before merging, OR add a comment in the workflow explaining why these tests are temporarily excluded, and create follow-up issues to track fixing these tests.

2. Credentials Management Verification

I verified that all three providers are properly registered in src/celeste/credentials.py (lines 63, 69, 76). The workflow changes align correctly with the credentials system.

---

Security Considerations

Good Practices:

• Secrets are stored in GitHub repository settings (not hardcoded)
• Using GitHub Actions secrets mechanism correctly
• No credentials exposed in code

Minor Concern:

• Ensure the three new secrets are actually configured in the repository integration-tests environment settings

---

Performance and Test Coverage

Concerns:

1. Reduced Test Coverage: The PR removes test coverage for all stream edit image tests and both Google Gemini 2.5 TTS models

2. Integration Test Reliability: The presence of failing tests suggests potential issues with the image streaming functionality and Google TTS models

Questions:

• Are the Google TTS failures related to the google-auth dependency addition?
• Has this been tested locally with the new API keys?
• What is the root cause of the test_stream_edit failures?

---

Recommendations

High Priority:

1. Add PR context explaining why tests are being excluded
2. Verify secrets configuration in repository settings
3. Consider splitting the PR (one for API keys, separate for test exclusions)

Medium Priority:
4. Add inline comments in workflow explaining temporary test exclusions
5. Consider version pinning for google-auth (e.g., google-auth>=2.0.0,<3.0.0)

---

Conclusion

Approval Status: Conditional Approval

The core changes (adding API keys and the google-auth dependency) are correct and necessary. However, the test exclusions are concerning and appear to be masking underlying issues.

Suggested Action:

• Merge if: Tests are known to be flaky/temporary issues and follow-up issues exist
• Block if: Test failures have not been investigated and could indicate real problems

The PR accomplishes its stated goal but raises questions about test reliability and hidden issues that should be addressed.