ci: reduce disk usage in unit tests (free space + test-only deps)

[manavgup]

linked to #411

[github-actions]

🚀 Development Environment Options

This repository supports Dev Containers for a consistent development environment.

Option 1: GitHub Codespaces (Recommended)

Create a cloud-based development environment:

1. Click the green Code button above
2. Select the Codespaces tab
3. Click Create codespace on fix/ci-disk-space-411
4. Wait 2-3 minutes for environment setup
5. Start coding with all tools pre-configured!

Option 2: VS Code Dev Containers (Local)

Use Dev Containers on your local machine:

1. Install Docker Desktop
2. Install VS Code
3. Install the Dev Containers extension
4. Clone this PR branch locally
5. Open in VS Code and click "Reopen in Container" when prompted

Option 3: Traditional Local Setup

Set up the development environment manually:

# Clone the repository
git clone https://github.com/manavgup/rag_modulo.git
cd rag_modulo
git checkout fix/ci-disk-space-411

# Initialize development environment
make dev-init
make dev-build
make dev-up
make dev-validate

Available Commands

Once in your development environment:

make help           # Show all available commands
make dev-validate   # Validate environment setup
make test-atomic    # Run atomic tests
make test-unit      # Run unit tests
make lint          # Run linting

Services Available

When running make dev-up:

• Backend API: http://localhost:8000
• Frontend: http://localhost:3000
• MLflow: http://localhost:5001

---

This automated message helps reviewers quickly set up the development environment.

[github-actions]

Claude Code Review

Summary

This PR optimizes the unit test workflow by reducing disk usage through parallel cleanup operations and installing only test dependencies. Overall approach is sound, but there is a CRITICAL ISSUE that will cause tests to fail.

---

CRITICAL ISSUE - Missing Runtime Dependencies for Tests

Line 94 uses: poetry install --only test --no-root --no-cache

PROBLEM: Using --only test installs ONLY the test group dependencies (pytest, coverage tools) but EXCLUDES all main project dependencies that tests import from. Tests import from rag_solution and will fail with ModuleNotFoundError.

SOLUTION: Use --with test instead to install main dependencies + test dependencies:

poetry install --with test --without dev --no-root --no-cache

This still achieves disk savings by skipping the heavy dev group (linters, type checkers, security scanners) while keeping the runtime dependencies tests need.

---

What Works Well

1. Parallel Cleanup Operations (lines 68-74) - Excellent use of background processes, removes ~10GB
2. Disk Usage Reporting (lines 65, 76) - Good observability for debugging
3. Smart Caching Strategy (lines 79-87) - Maintains Poetry cache for speed
4. No-Root Flag (line 94) - Appropriate for CI environment

---

Recommendations

1. Fix the Dependency Installation (HIGH PRIORITY) - Change --only test to --with test --without dev
2. Update Step Comment Numbers (LOW PRIORITY) - Lines 96-97 still say step 5 but should be step 6
3. Add Disk Usage Validation (OPTIONAL) - Consider adding a check to ensure at least 5GB free after cleanup

---

Testing Checklist

• Unit tests pass with fixed dependency installation
• Disk usage is reduced (check workflow logs)
• Build time is ~2-3 min (per CLAUDE.md target)
• No missing import errors in test output

---

Performance Analysis

Expected savings:

• Cleanup time: +30-60s (one-time cost)
• Install time: -60-90s (skipping dev deps)
• Net benefit: ~30-60s faster

Disk space: ~10GB freed (from 14GB to 24GB available)

---

Verdict

STATUS: REQUIRES CHANGES

The optimization strategy is excellent and aligns with fast PR feedback goals. However, the critical dependency issue must be fixed before merging.

Next steps:

1. Change --only test to --with test --without dev
2. Verify tests pass
3. Consider adding disk validation

Great work on identifying and addressing the disk usage issue!

[github-actions]

PR Review: CI Disk Space Optimization

Overview

This PR addresses disk space issues in the unit test workflow by adding cleanup steps and optimizing dependency installation.

Strengths

1. Targeted disk cleanup removing unnecessary tooling
2. Dependency optimization skipping dev group in CI
3. Safety validation with explicit disk space check
4. Clear documentation with helpful comments

Key Concerns

CRITICAL: --no-root Flag May Break Tests
The --no-root flag skips installing the project itself, which will likely break tests that import from rag_solution. This should be removed unless confirmed working.

Test Dependencies May Be Missing
Changing from --with dev,test to --with test --without dev could break tests that rely on dev tools like mypy, ruff, or pylint. Verify no tests import these.

--no-cache May Slow Builds
While --no-cache saves disk space, it forces Poetry to re-download packages on every run, potentially adding 30-60s to build time. Test without it first.

Hardcoded 5G Threshold
The 5G threshold is reasonable but arbitrary. Consider making it configurable.

Testing Recommendations

1. Run workflow on this PR and confirm tests pass
2. Compare build times before and after
3. Check if any tests use dev dependencies
4. Verify no import errors for rag_solution modules

Performance Impact

• Disk cleanup: ~10-15 GB freed
• Skipping dev deps: ~200-500 MB saved
• --no-cache: Saves disk but may add build time

Risk assessment: Medium - dependency changes could break tests

Security

No security concerns. Follows GitHub Actions best practices.

Final Verdict: Conditional Approval

The approach is sound but needs validation:

1. Remove --no-root flag (likely breaks imports)
2. Test without --no-cache first (may slow builds)
3. Confirm no tests depend on dev tools
4. Verify workflow passes before merging

Once validated, this is a good optimization for CI disk pressure.

---

Reviewed by: Claude Code
Date: 2025-10-14

[github-actions]

PR Review: CI Disk Space Optimization

I have reviewed PR #412 which addresses CI disk space issues. Here is my detailed feedback:

Strengths

1. Well-targeted optimization: The PR correctly identifies GitHub Actions runner disk space as a bottleneck and applies proven cleanup strategies.

2. Parallel deletion: Using background jobs with wait is an excellent optimization for removing large directories simultaneously.

3. Clear documentation: The echo statements provide good visibility into disk space before and after cleanup.

4. Appropriate dependency change: Installing only --with test --without dev is the right approach for unit tests. The test group already includes all necessary pytest dependencies.

5. Disk space validation: The threshold check is a smart safeguard to fail fast if cleanup is insufficient.

Potential Issues and Concerns

1. Critical: Missing Required Dependencies

The change from --with dev,test to --with test --without dev may break the workflow. According to pyproject.toml:

• Test group has: pytest, pytest-cov, pytest-html, etc.
• Dev group has: pytest-cov, pytest-html (duplicated), plus mypy, ruff, pylint

Problem: The test command uses --cov=rag_solution which requires pytest-cov. While pytest-cov IS in the test group (line 68), the duplication suggests historical dependency issues.

Recommendation: Monitor the first CI run carefully after merging. If coverage fails, we may need pytest-cov explicitly or revert to selective dev tool exclusion.

2. Variable Expansion Issue

Line 97 has the step name with a variable that will not expand. GitHub Actions does not interpolate step names, so this is cosmetic but misleading.

Fix: Use a literal value like Ensure sufficient free disk space or remove the threshold from the name.

3. Disk Space Check Logic

The comparison uses -lt (integer comparison) but the value extracted from df -BG could theoretically be a decimal. While df -BG typically rounds to integers, this could fail on edge cases.

Recommendation: Use integer arithmetic explicitly in the awk command with int().

4. Inconsistency with Other Workflows

I notice .github/workflows/01-lint.yml uses --only dev while 05-ci.yml uses --with dev,test. This PR creates a third pattern. While justified for disk constraints, it adds complexity.

Recommendation: Document this decision in a comment within the workflow file explaining why unit tests skip dev tools.

Performance Considerations

• Disk cleanup time: approximately 10-15 seconds (acceptable overhead)
• Dependency install time: Should be faster without dev tools (approximately 20-30% reduction)
• Overall impact: Net positive for CI runtime

Security

No security concerns. The sudo rm -rf commands target well-known GitHub Actions directories and pose no risk.

Test Coverage

Missing: No validation that unit tests will still pass with the new dependency configuration.

Recommendation:

1. Manually trigger the workflow after merge
2. Monitor for any pytest or coverage failures
3. Have a rollback plan ready

Alternative Approach for Future Consideration

Instead of manual cleanup, consider using GitHub free-disk-space action (jlumbroso/free-disk-space). This is more maintainable and battle-tested, though your custom script is fine for now.

---

Action Items Before Merge

• Fix variable expansion in step name (line 97)
• Add comment explaining dependency strategy (line 91-94)
• Consider integer casting in disk check (line 100)
• Plan to monitor first CI run after merge

Final Verdict: Approve with Minor Fixes

The core approach is solid and addresses a real problem. The suggested fixes are minor and could be addressed in a follow-up PR if needed. The disk space optimization is well-implemented and should resolve the CI failures.

Estimated disk savings: approximately 5-8GB (cleanup) + 1-2GB (dependencies) = 7-10GB total

Great work on identifying and fixing this bottleneck!

[github-actions]

PR Review: CI Disk Space Optimization

Summary

This PR addresses disk space constraints in GitHub Actions runners by implementing cleanup steps and optimizing dependency installation.

---

Strengths

1. Effective Disk Space Management - Parallel cleanup of large packages is a proven pattern
2. Smart Dependency Optimization - Skipping dev group in unit tests is logical since linters run in separate workflows
3. Good Documentation - Clear comments explain the rationale

---

Critical Issues

1. Disk Check Math Error (Line 105)

Problem: The integer truncation will cause false failures. Using int() in awk truncates decimals, so 4.9G becomes 4, failing a 5G threshold.

Current code:

FREE=$(df -BG / | awk 'NR==2 {gsub("G","",$4); print int($4)}')

Fix: Use print $4+0 to keep as float, or print int($4+0.5) for ceiling rounding.

2. Environment Variable Not Defined

MIN_FREE_GB is used but never set in the workflow. The default value of 5 will always be used.

Fix: Add to top-level env section:

env:
  MIN_FREE_GB: 5  # Minimum free disk space in GB

---

Additional Recommendations

3. Parallel Cleanup: Add existence checks before rm to avoid errors if directories do not exist
4. Step Numbering: Renumber steps for consistency after inserting step 3
5. Disk Threshold: Consider 7-10GB instead of 5GB for safety margin with temp files

---

Testing Considerations

• Verified pytest-cov is in test group ✅
• Monitor CI logs for missing dependencies
• Test with higher threshold to ensure stability

---

Security & Performance

• ✅ No security concerns - only removes unused system packages
• ✅ Parallel cleanup improves speed (~5-10s faster)
• ✅ Reduced dependency install time (~20-30% faster without dev tools)

---

Overall Assessment

Rating: ⭐⭐⭐⭐ (4/5)

Well-thought-out optimization with sound approach and clear documentation. However, the disk check math bug could cause flaky CI failures.

Recommendation: Request changes for items 1-2, then approve. Once fixed, this will be a solid improvement to CI reliability.

---

Great work on optimizing the CI pipeline! 🚀

[github-actions]

PR Review: CI Disk Space Optimization

This PR addresses disk space issues in the unit test CI workflow. The changes are well-structured but need some fixes before merging.

Strengths

1. Comprehensive disk cleanup removes large unnecessary packages in parallel
2. Smart dependency management skips dev group to reduce disk usage
3. Validation step with configurable threshold (MIN_FREE_GB=7)
4. Good inline documentation

Critical Issues (Blocking)

1. Potential Coverage Regression
Line 96-100 uses poetry install --with test --without dev. The inline comment suggests pytest-cov might not work, but according to pyproject.toml lines 66-76, pytest-cov IS in the test group.

Action needed:

• Verify pytest-cov is installed with this command
• Add verification step in the workflow
• Remove the speculative comment if it works

2. Wrong Issue Reference
PR body says linked to #411 but issue #411 is about Terraform/Ansible deployment architecture, not CI disk space.

Action needed: Fix or remove the issue reference

3. Inconsistent Threshold
ENV sets MIN_FREE_GB=7 but script defaults to 5GB in the validation step.

Action needed: Change script default to match 7GB

Non-Blocking Recommendations

4. Add error handling to parallel cleanup
5. Log disk usage at each step for debugging
6. Add more context to PR description about what was failing and why 7GB
7. Consider using the actions/free-disk-space action instead of custom script

Security and Performance

No security issues detected. Performance impact is positive (10-20s faster, smaller footprint).

Final Verdict

Approve with Changes - The approach is sound but the 3 blocking issues must be fixed before merge. Good work identifying and solving the disk space problem!

cc: @manavgup - Please address the blocking issues and confirm pytest-cov works with the new dependency installation.