Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/workflows/pythonpackage.yml
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ jobs:
GOOGLE_SA_FILE_B64: ${{ secrets.GOOGLE_SA_FILE_B64 }}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

[Contextual Comment]
This comment refers to code near real line 83. Anchored to nearest_changed(95) line 95.


P2 | Confidence: High

The addition of UPLIFTAI_KEY follows the existing pattern for secret management, which is good. However, the workflow does not include any validation to ensure required secrets are present before attempting tests. If a secret is missing (e.g., not configured in GitHub), the tests will fail with cryptic errors rather than clear feedback about missing environment variables. This could slow down troubleshooting for maintainers.

Code Suggestion:

Consider adding a pre-test step to validate that all required secrets are non-empty, or implement graceful skipping of tests for missing credentials with clear log messages.

PLAYHT_API_KEY: ${{ secrets.PLAYHT_API_KEY }}
PLAYHT_USER_ID: ${{ secrets.PLAYHT_USER_ID }}
UPLIFTAI_KEY: ${{ secrets.UPLIFTAI_KEY }}
steps:
- name: Checkout repo
uses: actions/checkout@v4
Expand Down
21 changes: 21 additions & 0 deletions AGENTS.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
# Instructions for Contributors

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 | Confidence: Medium

The documentation correctly captures the development workflow but lacks specific guidance on handling missing credentials. The test command uses -m "not synthetic and not sapi and not watson" to exclude certain engines, but contributors might not understand why these are excluded or how to handle other engines with missing credentials. This could lead to confusion when adding new engines or troubleshooting test failures. The note about GitHub Actions secrets is good but should be complemented with local development guidance.


- Use [`uv`](https://github.com/astral-sh/uv) for dependency management.
Install all extras with:

```bash
uv sync --all-extras
```

- Run tests with:

```bash
uv run pytest -m "not synthetic and not sapi and not watson"
```

- The test suite requires credentials for several TTS engines. Provide

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 | Confidence: High

This instruction is clear but incomplete. It doesn't specify whether contributors should also update the test command's filter flags (e.g., -m "not synthetic and not sapi and not watson") when adding new engines. This could lead to inconsistent test coverage if new engines are accidentally excluded from CI runs due to outdated filter flags.

Code Suggestion:

Expand the documentation to include guidance on updating test filters when adding new engines, and consider linking to the pytest marker definitions for clarity.

the necessary environment variables or a `credentials.json` file as
described in `tests/load_credentials.py`.

- When new engines require credentials, ensure corresponding secrets are
added to the GitHub Actions workflow environment variables.
Loading