-
Notifications
You must be signed in to change notification settings - Fork 2
Adds Github action to trigger tests upon raising PR #78
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: beta
Are you sure you want to change the base?
Changes from all commits
cc4d0cc
53a2b51
6f2aa9f
4b7da45
54f478f
a188398
372abf7
a810f57
fddb88a
a8faa04
a5e30ba
6204fd8
3159809
6c450e9
7d1db2f
0e49b4e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,165 @@ | ||||||||||||||||||||||||||||||||||||||
| name: Run Tests | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| on: | ||||||||||||||||||||||||||||||||||||||
| push: | ||||||||||||||||||||||||||||||||||||||
| branches: [main,beta] | ||||||||||||||||||||||||||||||||||||||
| pull_request: | ||||||||||||||||||||||||||||||||||||||
| branches: [main,beta] | ||||||||||||||||||||||||||||||||||||||
| types: [opened, synchronize, reopened, ready_for_review] | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| permissions: | ||||||||||||||||||||||||||||||||||||||
| contents: read | ||||||||||||||||||||||||||||||||||||||
| checks: write | ||||||||||||||||||||||||||||||||||||||
| pull-requests: write | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| jobs: | ||||||||||||||||||||||||||||||||||||||
| test-main: | ||||||||||||||||||||||||||||||||||||||
| name: Test Main SDK (Python 3.9) | ||||||||||||||||||||||||||||||||||||||
| runs-on: ubuntu-latest | ||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+16
to
+18
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Add workflow-level hardening and concurrency. Reduces default token scope and auto-cancels superseded runs on the same branch. jobs:
+ # Cancel previous in-progress runs for the same ref
+ # and restrict default permissions
+ concurrency:
+ group: ${{ github.workflow }}-${{ github.ref }}
+ cancel-in-progress: true
+
+permissions:
+ contents: read
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||
| if: github.event.pull_request.draft == false | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+19
to
+20
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Draft/fork-safe gating: keep runs on push, skip drafts and forks on PRs. Current condition breaks on push events. Use an event-aware expression. - if: github.event.pull_request.draft == false
+ if: ${{ github.event_name != 'pull_request' || (github.event.pull_request.draft == false && github.event.pull_request.head.repo.fork == false) }}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||
| steps: | ||||||||||||||||||||||||||||||||||||||
| - uses: actions/checkout@v4 | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| - name: Backup pyproject.toml | ||||||||||||||||||||||||||||||||||||||
| run: | | ||||||||||||||||||||||||||||||||||||||
| cp pyproject.toml pyproject.toml.bak | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| - name: Remove additional dependencies | ||||||||||||||||||||||||||||||||||||||
| run: | | ||||||||||||||||||||||||||||||||||||||
| sed -i.bak '/^additional_dev = \[$/,/]$/d' pyproject.toml | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| - name: Install uv | ||||||||||||||||||||||||||||||||||||||
| uses: astral-sh/setup-uv@v4 | ||||||||||||||||||||||||||||||||||||||
| with: | ||||||||||||||||||||||||||||||||||||||
| version: "latest" | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+32
to
+36
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Pin uv installer version for reproducibility. Avoid “latest” drift. - - name: Install uv
- uses: astral-sh/setup-uv@v4
- with:
- version: "latest"
+ - name: Install uv
+ uses: astral-sh/setup-uv@v4
+ with:
+ version: "0.5.x"Also applies to: 107-111 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||
| - name: Set up Python 3.9 | ||||||||||||||||||||||||||||||||||||||
| run: uv python install 3.9 | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
SamstyleGhost marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||
| - name: Install dependencies (dev only) | ||||||||||||||||||||||||||||||||||||||
| run: | | ||||||||||||||||||||||||||||||||||||||
| uv sync --python 3.9 | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+40
to
+43
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Install only dev dependencies and lock resolution for reproducibility. Use group selection and --frozen to catch accidental resolver drift. - - name: Install dependencies (dev only)
- run: |
- uv sync --python 3.9
+ - name: Install dependencies (dev only)
+ run: |
+ uv sync --group dev --python 3.9.20 --frozen📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
Comment on lines
+37
to
+43
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Pin Python patch and install dev-only with frozen lock. Improves determinism; avoids pulling unintended extras. - - name: Set up Python 3.9
- run: uv python install 3.9
+ - name: Set up Python 3.9
+ run: uv python install 3.9.20
@@
- - name: Install dependencies (dev only)
- run: |
- uv sync --python 3.9
+ - name: Install dependencies (dev only)
+ run: |
+ uv sync --group dev --python 3.9.20 --frozen- - name: Set up Python 3.10
- run: uv python install 3.10
+ - name: Set up Python 3.10
+ run: uv python install 3.10.15
@@
- - name: Install dependencies (CrewAI only)
- run: |
- uv sync --python 3.10
+ - name: Install dependencies (dev only)
+ run: |
+ uv sync --group dev --python 3.10.15 --frozenAlso applies to: 112-118 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||
| - name: Run main tests (excluding CrewAI) | ||||||||||||||||||||||||||||||||||||||
| env: | ||||||||||||||||||||||||||||||||||||||
| MAXIM_API_KEY: ${{ secrets.MAXIM_API_KEY }} | ||||||||||||||||||||||||||||||||||||||
| MAXIM_BASE_URL: ${{ secrets.MAXIM_BASE_URL }} | ||||||||||||||||||||||||||||||||||||||
| MAXIM_DATASET_ID: ${{ secrets.MAXIM_DATASET_ID }} | ||||||||||||||||||||||||||||||||||||||
| MAXIM_WORKSPACE_ID: ${{ secrets.MAXIM_WORKSPACE_ID }} | ||||||||||||||||||||||||||||||||||||||
| MAXIM_LOG_REPO_ID: ${{ secrets.MAXIM_LOG_REPO_ID }} | ||||||||||||||||||||||||||||||||||||||
| MAXIM_PROMPT_CHAIN_VERSION_ID: ${{ secrets.MAXIM_PROMPT_CHAIN_VERSION_ID }} | ||||||||||||||||||||||||||||||||||||||
| MAXIM_ASSISTANT_PROMPT_VERSION_ID: ${{ secrets.MAXIM_ASSISTANT_PROMPT_VERSION_ID }} | ||||||||||||||||||||||||||||||||||||||
| MAXIM_ASSISTANT_PROMPT_CHAIN_VERSION_ID: ${{ secrets.MAXIM_ASSISTANT_PROMPT_CHAIN_VERSION_ID }} | ||||||||||||||||||||||||||||||||||||||
| OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }} | ||||||||||||||||||||||||||||||||||||||
| ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }} | ||||||||||||||||||||||||||||||||||||||
| AZURE_OPENAI_ENDPOINT: ${{ secrets.AZURE_OPENAI_ENDPOINT }} | ||||||||||||||||||||||||||||||||||||||
| AZURE_OPENAI_KEY: ${{ secrets.AZURE_OPENAI_KEY }} | ||||||||||||||||||||||||||||||||||||||
| PORTKEY_API_KEY: ${{ secrets.PORTKEY_API_KEY }} | ||||||||||||||||||||||||||||||||||||||
| PORTKEY_VIRTUAL_KEY: ${{ secrets.PORTKEY_VIRTUAL_KEY }} | ||||||||||||||||||||||||||||||||||||||
| LLAMAINDEX_API_KEY: ${{ secrets.LLAMAINDEX_API_KEY }} | ||||||||||||||||||||||||||||||||||||||
| TOGETHER_API_KEY: ${{ secrets.TOGETHER_API_KEY }} | ||||||||||||||||||||||||||||||||||||||
| TAVILY_API_KEY: ${{ secrets.TAVILY_API_KEY }} | ||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+58
to
+62
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Consider removing unused secrets (PORTKEY_VIRTUAL_KEY). Tests moved to provider-based config; if PORTKEY_VIRTUAL_KEY is unused, omit it from env to reduce exposure surface. Do you want me to scan the repo for usages and submit a follow-up patch? 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||
| FIREWORKS_API_KEY: ${{ secrets.FIREWORKS_API_KEY }} | ||||||||||||||||||||||||||||||||||||||
| GROQ_API_KEY: ${{ secrets.GROQ_API_KEY }} | ||||||||||||||||||||||||||||||||||||||
| GOOGLE_API_KEY: ${{ secrets.GOOGLE_API_KEY }} | ||||||||||||||||||||||||||||||||||||||
| MAXIM_PROMPT_1_ID: ${{ secrets.MAXIM_PROMPT_1_ID }} | ||||||||||||||||||||||||||||||||||||||
| MAXIM_PROMPT_2_ID: ${{ secrets.MAXIM_PROMPT_2_ID }} | ||||||||||||||||||||||||||||||||||||||
| MAXIM_PROMPT_3_ID: ${{ secrets.MAXIM_PROMPT_3_ID }} | ||||||||||||||||||||||||||||||||||||||
| MAXIM_PROMPT_1_VERSION_1_ID: ${{ secrets.MAXIM_PROMPT_1_VERSION_1_ID }} | ||||||||||||||||||||||||||||||||||||||
| MAXIM_PROMPT_1_VERSION_3_ID: ${{ secrets.MAXIM_PROMPT_1_VERSION_3_ID }} | ||||||||||||||||||||||||||||||||||||||
| MAXIM_PROMPT_1_VERSION_4_ID: ${{ secrets.MAXIM_PROMPT_1_VERSION_4_ID }} | ||||||||||||||||||||||||||||||||||||||
| MAXIM_PROMPT_1_VERSION_5_ID: ${{ secrets.MAXIM_PROMPT_1_VERSION_5_ID }} | ||||||||||||||||||||||||||||||||||||||
| MAXIM_FOLDER_1_ID: ${{ secrets.MAXIM_FOLDER_1_ID }} | ||||||||||||||||||||||||||||||||||||||
| MAXIM_FOLDER_2_ID: ${{ secrets.MAXIM_FOLDER_2_ID }} | ||||||||||||||||||||||||||||||||||||||
| PROMPT_VERSION_ID: ${{ secrets.PROMPT_VERSION_ID }} | ||||||||||||||||||||||||||||||||||||||
| FOLDER_ID: ${{ secrets.FOLDER_ID }} | ||||||||||||||||||||||||||||||||||||||
| MAXIM_PROMPT_3_VERSION_2_ID: ${{ secrets.MAXIM_PROMPT_3_VERSION_2_ID }} | ||||||||||||||||||||||||||||||||||||||
| MAXIM_TEST_RUN_PROMPT_VERSION_ID: ${{ secrets.MAXIM_TEST_RUN_PROMPT_VERSION_ID }} | ||||||||||||||||||||||||||||||||||||||
| MAXIM_TEST_RUN_ASSISTANT_PROMPT_VERSION_ID: ${{ secrets.MAXIM_TEST_RUN_ASSISTANT_PROMPT_VERSION_ID }} | ||||||||||||||||||||||||||||||||||||||
| MAXIM_TEST_RUN_ASSISTANT_PROMPT_CHAIN_VERSION_ID: ${{ secrets.MAXIM_TEST_RUN_ASSISTANT_PROMPT_CHAIN_VERSION_ID }} | ||||||||||||||||||||||||||||||||||||||
| run: | | ||||||||||||||||||||||||||||||||||||||
| uv run pytest -v maxim/tests/test_prompts.py maxim/tests/test_openai.py maxim/tests/test_llamaindex.py maxim/tests/test_together.py maxim/tests/test_fireworks.py maxim/tests/test_groq.py maxim/tests/test_add_dataset_entries_comprehensive.py maxim/tests/test_maxim_core_simple.py --junitxml=junit/main-test-results.xml | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| - name: Debug environment | ||||||||||||||||||||||||||||||||||||||
| run: | | ||||||||||||||||||||||||||||||||||||||
| echo "Event name: ${{ github.event_name }}" | ||||||||||||||||||||||||||||||||||||||
| echo "PR from fork: ${{ github.event.pull_request.head.repo.full_name != github.repository }}" | ||||||||||||||||||||||||||||||||||||||
| echo "Base ref: ${{ github.base_ref }}" | ||||||||||||||||||||||||||||||||||||||
| echo "Head ref: ${{ github.head_ref }}" | ||||||||||||||||||||||||||||||||||||||
| echo "Repository: ${{ github.repository }}" | ||||||||||||||||||||||||||||||||||||||
| # Test if secrets are available (without exposing them) | ||||||||||||||||||||||||||||||||||||||
| if [ -z "${{ secrets.OPENAI_API_KEY }}" ]; then | ||||||||||||||||||||||||||||||||||||||
| echo "OPENAI_API_KEY is not available" | ||||||||||||||||||||||||||||||||||||||
| else | ||||||||||||||||||||||||||||||||||||||
| echo "OPENAI_API_KEY is available" | ||||||||||||||||||||||||||||||||||||||
| fi | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| - name: Upload main test results | ||||||||||||||||||||||||||||||||||||||
| uses: actions/upload-artifact@v4 | ||||||||||||||||||||||||||||||||||||||
| if: success() || failure() # run this step even if previous step failed | ||||||||||||||||||||||||||||||||||||||
| with: | ||||||||||||||||||||||||||||||||||||||
| name: main-test-results | ||||||||||||||||||||||||||||||||||||||
| path: junit/main-test-results.xml | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| - name: Publish main test results | ||||||||||||||||||||||||||||||||||||||
| uses: EnricoMi/publish-unit-test-result-action@v2 | ||||||||||||||||||||||||||||||||||||||
| if: success() || failure() # run this step even if previous step failed | ||||||||||||||||||||||||||||||||||||||
| with: | ||||||||||||||||||||||||||||||||||||||
| junit_files: "junit/main-test-results.xml" | ||||||||||||||||||||||||||||||||||||||
| check_name: "Main Test Results" | ||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+98
to
+110
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Use always() for publishing steps (and consider pinning action SHAs). Ensures artifacts/checks publish on failure/cancel. - if: success() || failure() # run this step even if previous step failed
+ if: always()Apply to both upload-artifact and publish-unit-test-result steps in both jobs. Also consider pinning actions to commit SHAs for supply-chain safety. Also applies to: 124-136 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+98
to
+111
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Use always() for artifact publishing and pin actions to SHAs; grant checks: write. Ensures publishing even on cancelled and satisfies permission needs. - - name: Upload main test results
- uses: actions/upload-artifact@v4
- if: success() || failure() # run this step even if previous step failed
+ - name: Upload main test results
+ uses: actions/upload-artifact@<sha-for-v4>
+ if: always()
@@
- - name: Publish main test results
- uses: EnricoMi/publish-unit-test-result-action@v2
- if: success() || failure() # run this step even if previous step failed
+ - name: Publish main test results
+ uses: EnricoMi/publish-unit-test-result-action@<sha-for-v2>
+ if: always()
with:
junit_files: "junit/main-test-results.xml"
check_name: "Main Test Results"Add job-level permissions to allow creating check runs: test-main:
name: Test Main SDK (Python 3.9)
runs-on: ubuntu-latest
+ permissions:
+ contents: read
+ checks: write
+ pull-requests: write
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||
| - name: Restore pyproject.toml | ||||||||||||||||||||||||||||||||||||||
| run: | | ||||||||||||||||||||||||||||||||||||||
| mv pyproject.toml.bak pyproject.toml | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
coderabbitai[bot] marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||||||||||||||||||||||
| additional-tests: | ||||||||||||||||||||||||||||||||||||||
| name: Test Additional Integrations (Python 3.10) | ||||||||||||||||||||||||||||||||||||||
| runs-on: ubuntu-latest | ||||||||||||||||||||||||||||||||||||||
| if: github.event.pull_request.draft == false | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+117
to
+120
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Apply the same draft/fork gating to additional-tests. Mirror the main job condition. - if: github.event.pull_request.draft == false
+ if: ${{ github.event_name != 'pull_request' || (github.event.pull_request.draft == false && github.event.pull_request.head.repo.fork == false) }}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||
| steps: | ||||||||||||||||||||||||||||||||||||||
| - uses: actions/checkout@v4 | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| - name: Install uv | ||||||||||||||||||||||||||||||||||||||
| uses: astral-sh/setup-uv@v4 | ||||||||||||||||||||||||||||||||||||||
| with: | ||||||||||||||||||||||||||||||||||||||
| version: "latest" | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| - name: Backup .python-version | ||||||||||||||||||||||||||||||||||||||
| run: | | ||||||||||||||||||||||||||||||||||||||
| cp .python-version .python-version.bak | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| - name: Update .python-version for CrewAI | ||||||||||||||||||||||||||||||||||||||
| run: | | ||||||||||||||||||||||||||||||||||||||
| echo "3.10.0" > .python-version | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+129
to
+136
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Avoid editing .python-version in CI. These steps are brittle; remove them and install the desired Python directly. - - name: Backup .python-version
- run: |
- cp .python-version .python-version.bak
-
- - name: Update .python-version for CrewAI
- run: |
- echo "3.10.0" > .python-version
...
- - name: Restore .python-version
- if: always()
- run: |
- mv .python-version.bak .python-version
+ # No .python-version mutation requiredAlso applies to: 148-151 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||
| - name: Set up Python 3.10 | ||||||||||||||||||||||||||||||||||||||
| run: uv python install 3.10 | ||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+129
to
+138
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Remove .python-version hacks; install pinned 3.10 directly. - - name: Backup .python-version
- run: |
- cp .python-version .python-version.bak
-
- - name: Update .python-version for CrewAI
- run: |
- echo "3.10.0" > .python-version
-
- name: Set up Python 3.10
- run: uv python install 3.10
+ run: uv python install 3.10.15Also remove the final “Restore .python-version” step (see below). 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| - name: Install dependencies (CrewAI only) | ||||||||||||||||||||||||||||||||||||||
| run: | | ||||||||||||||||||||||||||||||||||||||
| uv sync --python 3.10 | ||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+137
to
+142
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Pin Python 3.10 patch; install dev-only with frozen; rename step. Increases determinism and clarity. - - name: Set up Python 3.10
- run: uv python install 3.10
+ - name: Set up Python 3.10
+ run: uv python install 3.10.15
@@
- - name: Install dependencies (CrewAI only)
- run: |
- uv sync --python 3.10
+ - name: Install dependencies (dev only)
+ run: |
+ uv sync --group dev --python 3.10.15 --frozen📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+137
to
+143
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ensure additional_dev deps are installed; pin Python and freeze. CrewAI lives in additional_dev; install both dev and additional_dev. - - name: Set up Python 3.10
- run: uv python install 3.10
+ - name: Set up Python 3.10
+ run: uv python install 3.10.15
- - name: Install dependencies (CrewAI only)
- run: |
- uv sync --python 3.10
+ - name: Install dependencies (dev + additional_dev)
+ run: |
+ uv sync --group dev --group additional_dev --python 3.10.15 --frozen📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
Comment on lines
+140
to
+143
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Install required groups for integrations with frozen resolution. - - name: Install dependencies (CrewAI only)
- run: |
- uv sync --python 3.10
+ - name: Install dependencies (dev + additional_dev)
+ run: |
+ uv sync --group dev --group additional_dev --python 3.10.15 --frozen📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||
| - name: Run additional integration tests | ||||||||||||||||||||||||||||||||||||||
| run: | | ||||||||||||||||||||||||||||||||||||||
| uv run pytest maxim/tests/test_crewai.py --junitxml=junit/additional-test-results.xml | ||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+144
to
+146
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Create junit dir and pass required env for integration tests. Prevents junit write failure; ensure secrets available if needed. - - name: Run additional integration tests
- run: |
- uv run pytest maxim/tests/test_crewai.py --junitxml=junit/additional-test-results.xml
+ - name: Run additional integration tests
+ env:
+ OPENAI_API_KEY: ${{ secrets.OPENAI_API_KEY }}
+ ANTHROPIC_API_KEY: ${{ secrets.ANTHROPIC_API_KEY }}
+ MAXIM_API_KEY: ${{ secrets.MAXIM_API_KEY }}
+ MAXIM_BASE_URL: ${{ secrets.MAXIM_BASE_URL }}
+ TAVILY_API_KEY: ${{ secrets.TAVILY_API_KEY }}
+ run: |
+ mkdir -p junit
+ uv run pytest maxim/tests/test_crewai.py --junitxml=junit/additional-test-results.xml📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| - name: Upload additional test results | ||||||||||||||||||||||||||||||||||||||
| uses: actions/upload-artifact@v4 | ||||||||||||||||||||||||||||||||||||||
| if: success() || failure() # run this step even if previous step failed | ||||||||||||||||||||||||||||||||||||||
| with: | ||||||||||||||||||||||||||||||||||||||
| name: additional-test-results | ||||||||||||||||||||||||||||||||||||||
| path: junit/additional-test-results.xml | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| - name: Publish additional test results | ||||||||||||||||||||||||||||||||||||||
| uses: EnricoMi/publish-unit-test-result-action@v2 | ||||||||||||||||||||||||||||||||||||||
| if: success() || failure() # run this step even if previous step failed | ||||||||||||||||||||||||||||||||||||||
| with: | ||||||||||||||||||||||||||||||||||||||
| junit_files: "junit/additional-test-results.xml" | ||||||||||||||||||||||||||||||||||||||
| check_name: "Additional Test Results" | ||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||
| - name: Restore .python-version | ||||||||||||||||||||||||||||||||||||||
| if: always() # run this step even if previous steps failed | ||||||||||||||||||||||||||||||||||||||
| run: | | ||||||||||||||||||||||||||||||||||||||
| mv .python-version.bak .python-version | ||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+162
to
+165
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Drop .python-version restore (no longer modified). - - name: Restore .python-version
- if: always() # run this step even if previous steps failed
- run: |
- mv .python-version.bak .python-version
+ # No restore needed📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -75,10 +75,16 @@ def parse_incoming_query(incoming_query: str) -> List[RuleType]: | |||||||||||||||
| value = bool(value) | ||||||||||||||||
| parsed = True | ||||||||||||||||
| if not parsed: | ||||||||||||||||
| if type(value) == bool: | ||||||||||||||||
| try: | ||||||||||||||||
| value = int(value) | ||||||||||||||||
| parsed = True | ||||||||||||||||
| except ValueError: | ||||||||||||||||
| pass | ||||||||||||||||
| if not parsed: | ||||||||||||||||
| if isinstance(value, bool): | ||||||||||||||||
| value = bool(value) | ||||||||||||||||
| parsed = True | ||||||||||||||||
| elif type(value) is int: | ||||||||||||||||
| elif isinstance(value, int): | ||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Fix inconsistent type checking pattern. Line 87 uses For consistency with the existing codebase pattern, apply this diff: - elif isinstance(value, int):
+ elif type(value) is int:📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||
| value = int(value) | ||||||||||||||||
| parsed = True | ||||||||||||||||
|
Comment on lines
+84
to
89
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Remove redundant type checks after parsing. Lines 84-89 contain redundant type checks that will never execute because Apply this diff to remove the redundant checks: - if not parsed:
- if isinstance(value, bool):
- value = bool(value)
- parsed = True
- elif isinstance(value, int):
- value = int(value)
- parsed = True📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||
| if not parsed: | ||||||||||||||||
|
Comment on lines
+78
to
90
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Be conservative when casting to int and remove unreachable isinstance branches
- if not parsed:
- try:
- value = int(value)
- parsed = True
- except ValueError:
- pass
- if not parsed:
- if isinstance(value, bool):
- value = bool(value)
- parsed = True
- elif isinstance(value, int):
- value = int(value)
- parsed = True
+ if not parsed and isinstance(value, str) and re.fullmatch(r"[+-]?\d+", value.strip()):
+ try:
+ value = int(value.strip())
+ parsed = True
+ except ValueError:
+ pass
🤖 Prompt for AI Agents |
||||||||||||||||
|
|
||||||||||||||||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -7,7 +7,11 @@ | |||||||||||||||||||||||
| from maxim import Maxim, Config | ||||||||||||||||||||||||
| from maxim.models.dataset import DatasetEntry, Variable, DatasetEntryWithRowNo, FileVariablePayload, VariableFileAttachment | ||||||||||||||||||||||||
| from maxim.logger.components.attachment import FileAttachment, FileDataAttachment, UrlAttachment | ||||||||||||||||||||||||
| from dotenv import load_dotenv | ||||||||||||||||||||||||
| load_dotenv() | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| MAXIM_BASE_URL = os.getenv("MAXIM_BASE_URL") | ||||||||||||||||||||||||
| MAXIM_API_KEY = os.getenv("MAXIM_API_KEY") | ||||||||||||||||||||||||
|
Comment on lines
+10
to
+14
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) 🛠️ Refactor suggestion Avoid module-scope dotenv loading and env caching; this breaks test hermeticity.
Apply this diff to make resolution explicit and add safe test defaults (while still honoring env when set): -from dotenv import load_dotenv
-load_dotenv()
-
-MAXIM_BASE_URL = os.getenv("MAXIM_BASE_URL")
-MAXIM_API_KEY = os.getenv("MAXIM_API_KEY")
+from dotenv import load_dotenv, find_dotenv
+# Resolve nearest .env without overriding already-set env
+load_dotenv(find_dotenv(usecwd=True), override=False)
+# Note: do not cache real secrets at import-time; provide safe test defaults
+MAXIM_BASE_URL = os.getenv("MAXIM_BASE_URL", "http://localhost:8000")
+MAXIM_API_KEY = os.getenv("MAXIM_API_KEY", "test-api-key")If you don’t want defaults, drop them here and inject via setUp using patch.dict. I can provide a version that fully avoids module-level reads. 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents |
||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| class TestAddDatasetEntriesComprehensive(unittest.TestCase): | ||||||||||||||||||||||||
| """Comprehensive test suite for the updated add_dataset_entries method.""" | ||||||||||||||||||||||||
|
|
@@ -17,13 +21,9 @@ def setUp(self) -> None: | |||||||||||||||||||||||
| if hasattr(Maxim, "_instance"): | ||||||||||||||||||||||||
| delattr(Maxim, "_instance") | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| # Set up test environment variables | ||||||||||||||||||||||||
| os.environ["MAXIM_API_KEY"] = "test-api-key" | ||||||||||||||||||||||||
| os.environ["MAXIM_BASE_URL"] = "https://app.getmaxim.ai" | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| config = Config( | ||||||||||||||||||||||||
| api_key="test-api-key", | ||||||||||||||||||||||||
| base_url="https://app.getmaxim.ai", | ||||||||||||||||||||||||
| api_key=MAXIM_API_KEY, | ||||||||||||||||||||||||
| base_url=MAXIM_BASE_URL, | ||||||||||||||||||||||||
| debug=True, | ||||||||||||||||||||||||
| raise_exceptions=True | ||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||
|
Comment on lines
24
to
29
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🛠️ Refactor suggestion Do not pass None into Config; load env in setUp with safe defaults Maxim requires an API key; without defaults, CI will fail. Build kwargs conditionally or provide test-safe defaults. Apply: - config = Config(
- api_key=MAXIM_API_KEY,
- base_url=MAXIM_BASE_URL,
- debug=True,
- raise_exceptions=True
- )
+ # Resolve env at setup time
+ load_dotenv(find_dotenv(usecwd=True), override=False)
+ api_key = os.getenv("MAXIM_API_KEY", "test-api-key")
+ base_url = os.getenv("MAXIM_BASE_URL", "http://localhost:8000")
+ cfg = {"debug": True, "raise_exceptions": True}
+ # Only add keys if present to avoid None-paths; we provide safe defaults above.
+ if api_key:
+ cfg["api_key"] = api_key
+ if base_url:
+ cfg["base_url"] = base_url
+ config = Config(**cfg)
🤖 Prompt for AI Agents |
||||||||||||||||||||||||
|
|
@@ -34,10 +34,6 @@ def tearDown(self) -> None: | |||||||||||||||||||||||
| """Clean up after tests.""" | ||||||||||||||||||||||||
| if hasattr(Maxim, "_instance"): | ||||||||||||||||||||||||
| delattr(Maxim, "_instance") | ||||||||||||||||||||||||
| if "MAXIM_API_KEY" in os.environ: | ||||||||||||||||||||||||
| del os.environ["MAXIM_API_KEY"] | ||||||||||||||||||||||||
| if "MAXIM_BASE_URL" in os.environ: | ||||||||||||||||||||||||
| del os.environ["MAXIM_BASE_URL"] | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| def _create_mock_response(self, content: bytes, headers: dict = None) -> Mock: | ||||||||||||||||||||||||
| """Create a mock HTTP response.""" | ||||||||||||||||||||||||
|
|
@@ -175,7 +171,7 @@ def test_add_dataset_entries_with_file_attachments(self) -> None: | |||||||||||||||||||||||
| name="test.txt", | ||||||||||||||||||||||||
| mime_type="text/plain" | ||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| entry = DatasetEntry(entry={ | ||||||||||||||||||||||||
| "input": Variable(type="text", payload="Input with file"), | ||||||||||||||||||||||||
| "files": Variable(type="file", payload=[file_attachment]) | ||||||||||||||||||||||||
|
|
@@ -186,7 +182,7 @@ def test_add_dataset_entries_with_file_attachments(self) -> None: | |||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| # Verify file upload process was triggered | ||||||||||||||||||||||||
| self.assertEqual(mock_client.request.call_count, 4) # total_rows + entries + upload_url + patch | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| finally: | ||||||||||||||||||||||||
| # Clean up temp file | ||||||||||||||||||||||||
| os.unlink(temp_file_path) | ||||||||||||||||||||||||
|
|
@@ -201,7 +197,7 @@ def test_add_dataset_entries_with_url_attachment(self) -> None: | |||||||||||||||||||||||
| b'{"data": {"url": "https://signed-url.com", "key": "datasets/test-dataset-id/entry123/test-file-key"}}' | ||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||
| mock_patch_response = self._create_mock_response(b'{"success": true}') | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| self._setup_mock_network_calls([ | ||||||||||||||||||||||||
| mock_total_rows_response, | ||||||||||||||||||||||||
| mock_add_entries_response, | ||||||||||||||||||||||||
|
|
@@ -215,7 +211,7 @@ def test_add_dataset_entries_with_url_attachment(self) -> None: | |||||||||||||||||||||||
| name="image.jpg", | ||||||||||||||||||||||||
| mime_type="image/jpeg" | ||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| entry = DatasetEntry(entry={ | ||||||||||||||||||||||||
| "input": Variable(type="text", payload="Input with URL"), | ||||||||||||||||||||||||
| "images": Variable(type="file", payload=[url_attachment]) | ||||||||||||||||||||||||
|
|
@@ -240,7 +236,7 @@ def test_add_dataset_entries_with_file_data_attachment(self) -> None: | |||||||||||||||||||||||
| b'{"data": {"url": "https://signed-url.com", "key": "datasets/test-dataset-id/entry123/test-file-key"}}' | ||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||
| mock_patch_response = self._create_mock_response(b'{"success": true}') | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| mock_client = self._setup_mock_network_calls([ | ||||||||||||||||||||||||
| mock_total_rows_response, | ||||||||||||||||||||||||
| mock_add_entries_response, | ||||||||||||||||||||||||
|
|
@@ -254,7 +250,7 @@ def test_add_dataset_entries_with_file_data_attachment(self) -> None: | |||||||||||||||||||||||
| name="data.bin", | ||||||||||||||||||||||||
| mime_type="application/octet-stream" | ||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| entry = DatasetEntry(entry={ | ||||||||||||||||||||||||
| "input": Variable(type="text", payload="Input with file data"), | ||||||||||||||||||||||||
| "data": Variable(type="file", payload=[file_data_attachment]) | ||||||||||||||||||||||||
|
|
@@ -264,15 +260,15 @@ def test_add_dataset_entries_with_file_data_attachment(self) -> None: | |||||||||||||||||||||||
| self.maxim.add_dataset_entries(self.dataset_id, [entry]) | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| # Verify the upload process was initiated | ||||||||||||||||||||||||
| self.assertEqual(mock_client.request.call_count, 4) | ||||||||||||||||||||||||
| self.assertEqual(mock_client.request.call_count, 2) | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
|
Comment on lines
262
to
264
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧹 Nitpick (assertive) Ruff PT009: keep or suppress consistently. File uses 🧰 Tools🪛 Ruff (0.12.2)263-263: Use a regular Replace (PT009) 🤖 Prompt for AI Agents |
||||||||||||||||||||||||
| def test_add_dataset_entries_mixed_input_types(self) -> None: | ||||||||||||||||||||||||
| """Test add_dataset_entries with mixed DatasetEntry and dict inputs.""" | ||||||||||||||||||||||||
| mock_total_rows_response = self._create_mock_response(b'{"data": 10}') | ||||||||||||||||||||||||
| mock_add_entries_response = self._create_mock_response( | ||||||||||||||||||||||||
| b'{"data": {"ids": ["entry1", "entry2"], "cells": []}}' | ||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| mock_client = self._setup_mock_network_calls([ | ||||||||||||||||||||||||
| mock_total_rows_response, | ||||||||||||||||||||||||
| mock_add_entries_response | ||||||||||||||||||||||||
|
|
@@ -283,7 +279,7 @@ def test_add_dataset_entries_mixed_input_types(self) -> None: | |||||||||||||||||||||||
| "input": Variable(type="text", payload="Object input"), | ||||||||||||||||||||||||
| "output": Variable(type="json", payload={"result": "object"}), | ||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
| dict_entry = { | ||||||||||||||||||||||||
| "input": "Dict input", | ||||||||||||||||||||||||
| "output": {"result": "dict"} | ||||||||||||||||||||||||
|
|
||||||||||||||||||||||||
Uh oh!
There was an error while loading. Please reload this page.