Conversation
WalkthroughThis PR enhances test infrastructure by adding Docker-based test validation in the publish workflow, configuring pytest's Python path, adding an explicit langchain-core dependency, and implementing a conftest hook to manage import resolution order during test execution. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/docker-publish.yml:
- Around line 78-86: The publish workflow step that runs the docker command uses
an unpinned pip install ("pip install -q pytest pytest-asyncio") inside the
docker run -c shell; change that install to use lower-bound version constraints
(e.g., "pytest>=..." and "pytest-asyncio>=...") consistent with the repo's
dependency pattern so the pytest toolchain is pinned during the runtime image
verification, keeping the rest of the docker run invocation (SHORT_SHA,
--entrypoint sh, and python -m pytest ... --collect-only) unchanged.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 67ac33c3-54f1-400f-890e-222c815a82d6
📒 Files selected for processing (4)
.github/workflows/docker-publish.ymlpyproject.tomlrequirements.txttests/conftest.py
| - name: Verify — pytest collection in runtime image | ||
| run: | | ||
| SHORT_SHA=$(echo "${{ github.sha }}" | cut -c1-7) | ||
| docker run --rm \ | ||
| -v "${{ github.workspace }}:/repo" \ | ||
| -w /repo \ | ||
| --entrypoint sh \ | ||
| "${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:sha-${SHORT_SHA}" \ | ||
| -c 'pip install -q pytest pytest-asyncio && python -m pytest tests/ --collect-only -q' |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Workflow installs of pytest =="
rg -n --type=yaml 'pip install .*pytest' .github/workflows
echo
echo "== Declared pytest dependencies in repo manifests =="
rg -n --iglob 'pyproject.toml' --iglob 'requirements*.txt' 'pytest(-asyncio)?'Repository: protoLabsAI/protoAgent
Length of output: 335
🏁 Script executed:
cat -n pyproject.tomlRepository: protoLabsAI/protoAgent
Length of output: 384
🏁 Script executed:
wc -l pyproject.toml && echo "---" && cat pyproject.tomlRepository: protoLabsAI/protoAgent
Length of output: 336
🏁 Script executed:
find . -type f \( -name "requirements*.txt" -o -name "setup.py" -o -name "setup.cfg" -o -name "poetry.lock" -o -name "package-lock.json" -o -name "Pipfile.lock" -o -name "pdm.lock" \) | head -20Repository: protoLabsAI/protoAgent
Length of output: 104
🏁 Script executed:
cat requirements.txtRepository: protoLabsAI/protoAgent
Length of output: 345
🏁 Script executed:
echo "== Checking other CI workflows for pytest usage ==" && \
find .github/workflows -name "*.yml" -o -name "*.yaml" | xargs grep -l pytest 2>/dev/null || echo "No other pytest usage found"Repository: protoLabsAI/protoAgent
Length of output: 153
Pin the pytest toolchain in this publish-time verification step.
Line 86 installs unpinned pytest and pytest-asyncio, making this main-branch publish workflow susceptible to nondeterministic failures from upstream releases. The repo uses lower-bound version constraints elsewhere (e.g., gradio>=5.0, httpx>=0.27); apply the same pattern here.
💡 Suggested change
- -c 'pip install -q pytest pytest-asyncio && python -m pytest tests/ --collect-only -q'
+ -c 'python -m pip install -q "pytest>=8" "pytest-asyncio>=0.24" && python -m pytest tests/ --collect-only -q'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| - name: Verify — pytest collection in runtime image | |
| run: | | |
| SHORT_SHA=$(echo "${{ github.sha }}" | cut -c1-7) | |
| docker run --rm \ | |
| -v "${{ github.workspace }}:/repo" \ | |
| -w /repo \ | |
| --entrypoint sh \ | |
| "${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:sha-${SHORT_SHA}" \ | |
| -c 'pip install -q pytest pytest-asyncio && python -m pytest tests/ --collect-only -q' | |
| - name: Verify — pytest collection in runtime image | |
| run: | | |
| SHORT_SHA=$(echo "${{ github.sha }}" | cut -c1-7) | |
| docker run --rm \ | |
| -v "${{ github.workspace }}:/repo" \ | |
| -w /repo \ | |
| --entrypoint sh \ | |
| "${{ env.REGISTRY }}/${{ env.IMAGE_NAME }}:sha-${SHORT_SHA}" \ | |
| -c 'python -m pip install -q "pytest>=8" "pytest-asyncio>=0.24" && python -m pytest tests/ --collect-only -q' |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/docker-publish.yml around lines 78 - 86, The publish
workflow step that runs the docker command uses an unpinned pip install ("pip
install -q pytest pytest-asyncio") inside the docker run -c shell; change that
install to use lower-bound version constraints (e.g., "pytest>=..." and
"pytest-asyncio>=...") consistent with the repo's dependency pattern so the
pytest toolchain is pinned during the runtime image verification, keeping the
rest of the docker run invocation (SHORT_SHA, --entrypoint sh, and python -m
pytest ... --collect-only) unchanged.
Fix pytest collection errors on test_memory_persistence and test_skill_index.
Addresses the bug found during v0.2.0 smoke testing where running pytest tests/ in a fresh Docker env fails at collection with:
ModuleNotFoundError: No module named langchain_core.language_models; langchain_core is not a package
Recovered by Ava via REST API after pipeline could not push the branch itself.
Summary by CodeRabbit
langchain-corewith minimum version requirement.