Skip to content

ci: add XAI_API_KEY to integration test workflow#51

Merged
Kamilbenkirane merged 1 commit intomainfrom
add-xai-api-key-to-workflow
Nov 24, 2025
Merged

ci: add XAI_API_KEY to integration test workflow#51
Kamilbenkirane merged 1 commit intomainfrom
add-xai-api-key-to-workflow

Conversation

@Kamilbenkirane
Copy link
Copy Markdown
Member

Add XAI_API_KEY environment variable to the integration-test job in publish.yml workflow to support XAI provider integration tests.

@Kamilbenkirane Kamilbenkirane merged commit 93bbc2b into main Nov 24, 2025
11 checks passed
@claude
Copy link
Copy Markdown

claude Bot commented Nov 24, 2025

PR Review: Add XAI_API_KEY to integration test workflow

✅ Summary

This PR adds the XAI_API_KEY environment variable to the integration-test job in the publish workflow, enabling XAI provider integration tests during the release process.


Code Quality & Best Practices ✅

Positive:

  • Consistency: The change follows the established pattern of other provider API keys (OPENAI_API_KEY, ANTHROPIC_API_KEY, GOOGLE_API_KEY, etc.) in lines 56-61
  • Alphabetical ordering concern: While the change is functionally correct, the API keys are currently listed in a specific order. XAI_API_KEY is added at the end after BYTEDANCE_API_KEY, which maintains insertion order but not alphabetical order
  • Completeness: The change aligns with:
    • XAI provider implementation in packages/text-generation/src/celeste_text_generation/providers/xai/
    • XAI credential handling in src/celeste/credentials.py:40
    • XAI integration tests in packages/text-generation/tests/integration_tests/
    • XAI entry in .env.example:29

Potential Issues ✅

None identified - This is a straightforward configuration change with no code logic.


Security Considerations ✅

Secure implementation:

  • ✅ Uses GitHub Secrets (${{ secrets.XAI_API_KEY }}) - credentials are not hardcoded
  • ✅ Runs in the integration-tests environment (line 48), providing proper environment isolation
  • ✅ Follows the same security pattern as all other provider API keys
  • ✅ The secret needs to be configured in GitHub repository settings under the integration-tests environment

Action required:

  • Ensure XAI_API_KEY secret is configured in GitHub repository settings → Secrets and variables → Actions → Environment secrets → integration-tests

Performance Considerations ✅

No performance impact:

  • Adding an environment variable has negligible overhead
  • Integration tests already run in parallel (--dist=worksteal -n auto per Makefile:44)
  • XAI tests are part of the existing parametrized test suite

Test Coverage ✅

Well covered:

  • ✅ XAI provider is included in integration tests:
    • test_generate.py - line 15: (Provider.XAI, "grok-3-mini", {})
    • test_stream.py - includes XAI streaming tests
  • ✅ Without this change, XAI integration tests would fail during the publish workflow due to missing credentials
  • ✅ This change is necessary for the existing XAI integration tests to pass in the publish workflow

Additional Observations

  1. Missing from CI workflow: The ci.yml workflow does not run integration tests (only unit tests), so this change appropriately targets only the publish.yml workflow where integration tests are executed before release

  2. Consistency check: All providers with integration tests now have their API keys configured:

    • Text generation: OpenAI, Anthropic, Google, Mistral, Cohere, Bytedance, XAI ✅
    • Credentials mapping: All providers in PROVIDER_CREDENTIAL_MAP

Recommendation

LGTM! ✅ Approve and merge

This is a necessary and well-implemented change that:

  • Enables XAI integration testing in the publish workflow
  • Follows security best practices
  • Maintains consistency with other provider configurations
  • Has no side effects or risks

Before merging: Verify that the XAI_API_KEY secret is configured in the GitHub repository's integration-tests environment.

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