Reduce PR Iteration Time: Too Many CI Failures Before Success

[manavgup]

Reduce PR Iteration Time: Too Many CI Failures Before Success

Problem Statement

Currently, it takes 6-8 iterations for a PR to pass CI, which is inefficient and frustrating. The industry standard is 2-4 iterations, and we should aim for ~3 commits to green CI.

Recent Example (PR #506)

Test fixes after PYTHONPATH removal required multiple iterations:

1. Initial commit: Test fixes
2. Pre-commit hook failures (ruff config paths)
3. YAML syntax errors in pre-commit config
4. Secrets detection failures
5. Ansible-lint broken
6. Playwright import errors in CI
7. Final success

Total time wasted: ~45 minutes across iterations

Root Causes

1. The "Unseen State" Problem

Issues only surface at runtime:

• Pre-commit hooks have specific configurations
• CI environment differs from local (Python versions, dependencies, paths)
• Some hooks are broken (ansible-lint) but hidden until commit
• Test collection happens differently in CI vs local

2. The "Cascade Effect"

Fix tests → Commit → Pre-commit fails → Fix config → Commit → New issue → Repeat

Each fix reveals a new problem because:

• Can't run pre-commit hooks before committing (chicken-and-egg)
• Each hook has different requirements/environments
• 3 different environments with different failure modes:
  • Local development
  • Pre-commit isolated virtualenvs
  • GitHub Actions CI

3. No Pre-flight Checks

We lack automated checks that run before attempting to commit, leading to:

• Discovering issues one at a time
• Repeated commit → fail → fix → commit cycles
• Wasted time waiting for CI

Proposed Solutions

Strategy 1: Pre-flight Check Script (Priority: HIGH)

Create .local/pre-commit-check.sh:

#!/bin/bash
set -e

echo "🔍 Running pre-flight checks..."

# 1. Run all test suites
echo "1️⃣ Running tests..."
make test-unit-fast || exit 1
make test-atomic || exit 1
make test-integration || exit 1

# 2. Run pre-commit on all files
echo "2️⃣ Running pre-commit checks..."
pre-commit run --all-files || echo "⚠️  Pre-commit issues found - review above"

# 3. Check for common issues
echo "3️⃣ Checking for common issues..."
echo "  Checking for backend. prefix in tests..."
if grep -r "backend\.rag_solution" tests/ 2>/dev/null; then
    echo "  ❌ Found backend. prefix in test files"
    exit 1
fi

echo "  Checking for PYTHONPATH in Makefile..."
if grep "PYTHONPATH" Makefile 2>/dev/null; then
    echo "  ❌ PYTHONPATH still in Makefile"
    exit 1
fi

# 4. Check pytest can collect all tests
echo "4️⃣ Checking pytest collection..."
poetry run pytest --collect-only tests/ -q || exit 1

# 5. Validate YAML configs
echo "5️⃣ Validating configs..."
pre-commit validate-config || exit 1
python3 -c "import yaml; yaml.safe_load(open('.pre-commit-config.yaml'))" || exit 1

echo "✅ All pre-flight checks passed! Safe to commit."

Usage:

chmod +x .local/pre-commit-check.sh
./.local/pre-commit-check.sh  # Run before every commit

Benefits:

• Catches 80% of issues before committing
• Runs in 2-3 minutes (faster than commit → CI → fix cycle)
• Can be added to git hooks or Makefile

Strategy 2: Local CI Testing with act (Priority: MEDIUM)

Install and use act to run GitHub Actions locally:

# Install act (macOS)
brew install act

# Run CI pipeline locally BEFORE pushing
act -j pytest              # Run pytest job
act -j lint                # Run lint job
act -j security            # Run security job

# Run all jobs
act

Benefits:

• Catches CI-specific issues before pushing
• Tests in same environment as GitHub Actions
• Saves CI minutes and iteration time

Strategy 3: Incremental Commits with Draft PRs (Priority: MEDIUM)

Change workflow to fail fast:

# Old way: Try to get everything perfect, then commit
# New way: Commit early, iterate in draft PR

# 1. Make changes, commit immediately
git add tests/
git commit -m "fix: test fixes (WIP)"
git push origin feature-branch

# 2. Create DRAFT PR immediately
gh pr create --draft --title "[WIP] Test fixes"

# 3. Let CI run, see what breaks
# 4. Fix CI issues in separate small commits
git commit -m "fix(ci): exclude playwright"
git push

# 5. Mark ready when green
gh pr ready

Benefits:

• See CI failures incrementally
• Smaller commits easier to debug
• Less "surprise" issues at the end
• CI runs while you work on other things

Strategy 4: CI Simulation Make Target (Priority: HIGH)

Add to Makefile:

.PHONY: ci-local
ci-local: clean
	@echo "$(CYAN)🚀 Running full CI pipeline locally...$(NC)"
	@echo ""
	@echo "$(CYAN)1️⃣ Validating configuration...$(NC)"
	@pre-commit validate-config
	@poetry check --lock
	@echo "$(GREEN)✅ Config valid$(NC)"
	@echo ""
	@echo "$(CYAN)2️⃣ Running linters...$(NC)"
	@$(MAKE) lint
	@echo "$(GREEN)✅ Linting passed$(NC)"
	@echo ""
	@echo "$(CYAN)3️⃣ Running security checks...$(NC)"
	@$(MAKE) security-check
	@echo "$(GREEN)✅ Security passed$(NC)"
	@echo ""
	@echo "$(CYAN)4️⃣ Running test suite...$(NC)"
	@$(MAKE) test-atomic
	@$(MAKE) test-unit-fast
	@$(MAKE) test-integration
	@echo "$(GREEN)✅ All tests passed$(NC)"
	@echo ""
	@echo "$(CYAN)5️⃣ Checking pytest collection...$(NC)"
	@poetry run pytest --collect-only tests/ -q
	@echo "$(GREEN)✅ Test collection passed$(NC)"
	@echo ""
	@echo "$(GREEN)✅✅✅ Local CI pipeline passed! Safe to push. ✅✅✅$(NC)"

Usage:

make ci-local  # Run before pushing

Strategy 5: Fix Broken Pre-commit Hooks (Priority: HIGH)

Current broken hooks need fixing:

1. ansible-lint: Missing ansible core dependency

   # Fix: Either install ansible-core in pre-commit env or skip for Python projects
   # Recommendation: Remove ansible-lint hook (not applicable to Python backend)

2. detect-secrets: Flags test fixtures as secrets

   # Fix: Update .secrets.baseline regularly
   detect-secrets scan --baseline .secrets.baseline
   git add .secrets.baseline

3. markdownlint: Line length too strict (80 chars)

   # Already fixed: .markdownlint.json with 120 char limit

Strategy 6: Pre-commit Staging Configuration (Priority: LOW)

Update .pre-commit-config.yaml to separate fast vs slow hooks:

# Fast hooks (run on commit)
- repo: https://github.com/pre-commit/pre-commit-hooks
  hooks:
    - id: trailing-whitespace
      stages: [commit]
    - id: end-of-file-fixer
      stages: [commit]

# Slow hooks (run on push)
- repo: https://github.com/astral-sh/ruff-pre-commit
  hooks:
    - id: ruff
      stages: [push]  # Already configured
    - id: ruff-format
      stages: [push]

Implementation Plan

Phase 1: Quick Wins (1-2 hours)

• Create .local/pre-commit-check.sh script
• Add make ci-local target to Makefile
• Update .secrets.baseline to include test fixtures
• Remove broken ansible-lint hook from pre-commit config
• Document in CLAUDE.md: "Always run make ci-local before pushing"

Phase 2: Environment Improvements (2-3 hours)

• Install and configure act for local GitHub Actions testing
• Create .actrc configuration file
• Add make test-ci-local using act
• Test act with current CI workflows

Phase 3: Process Changes (Ongoing)

• Adopt "draft PR early, iterate in public" workflow
• Update contributing guide with new workflow
• Add pre-flight check to git pre-push hook (optional)
• Train team on new tools and processes

Phase 4: CI/CD Optimization (Separate Issue)

• Audit all GitHub Actions workflows for duplication
• Optimize parallel job execution
• Review caching strategies
• See issue #XXX for full CI/CD optimization

Success Metrics

Current State:

• Average iterations to green CI: 6-8
• Average time per PR: 45-60 minutes wasted on iterations
• Developer frustration: High

Target State:

• Average iterations to green CI: 2-3
• Average time per PR: 15-20 minutes (pre-flight check + 1-2 quick fixes)
• Developer frustration: Low
• Confidence before pushing: High

How to Measure:

• Track "commits per PR" metric
• Track "CI failure rate" (failures / total runs)
• Survey developer satisfaction quarterly

Related Issues

• Refactor E2E Test Targets for Clarity and Isolation #505 - E2E test refactoring
• Streamline Makefile: Remove outdated targets and adopt IBM MCP Context Forge best practices #348 - Makefile simplification
• #XXX - CI/CD optimization (to be created)

Questions for Discussion

1. Should we make make ci-local mandatory before pushing (pre-push hook)?
2. Should we invest in act for local CI testing or just improve pre-flight checks?
3. Should we adopt "draft PR early" as standard workflow?
4. Should we remove non-applicable hooks (ansible-lint) or fix them?

---

Priority: Medium
Effort: 4-6 hours total (spread across phases)
Impact: High - reduces developer friction, increases velocity, improves morale

[manavgup]

CI/CD Optimization Analysis

Executive Summary

Current State: RAG Modulo has a well-structured CI/CD pipeline with good separation of concerns
Main Issue: Some duplication and inefficiencies that increase PR iteration time
Recommendation: Consolidate duplicate steps, optimize caching, and streamline test execution

Analysis of Current Workflows

Workflow Inventory

1. 01-lint.yml - Lint & Static Analysis (runs on PR/push to main)
2. 02-security.yml - Security Scan (runs on PR/push to main)
3. 03-build-secure.yml - Secure Docker Build & Scan (post-merge only)
4. 04-pytest.yml - Unit Tests (runs on PR/push to main)
5. 05-ci.yml - Integration Tests & Build (push to main only)
6. 06-weekly-security-audit.yml - Weekly security audits
7. 07-frontend-lint.yml - Frontend linting
8. makefile-testing.yml - Makefile target validation
9. poetry-lock-check.yml - Poetry lock file validation
10. Other workflows: AI triage, Claude review, deployment, etc.

✅ Strengths

1. Good Separation of Concerns: Each workflow has a clear, single purpose
2. Concurrency Control: All main workflows use cancel-in-progress: true
3. Path Filtering: Smart triggering based on changed files
4. Matrix Strategy: Parallel execution for lint checks and security scans
5. Optimization Already Done: Docker builds moved post-merge (85% faster PRs)
6. Caching Strategy: Poetry dependencies and BuildKit cache implemented

❌ Issues & Duplications

1. Duplicate Dependency Installation (HIGH PRIORITY)

Problem: Every workflow installs Python/Poetry dependencies independently

Workflows affected:

• 01-lint.yml (installs poetry 4 times in matrix)
• 04-pytest.yml (full install)
• 05-ci.yml (full install with retry)
• makefile-testing.yml (full install)

Impact: ~30-60 seconds per workflow, multiplied across all jobs

Solution:

# Create reusable workflow: .github/workflows/setup-python-env.yml
name: Setup Python Environment
on:
  workflow_call:
    inputs:
      python-version:
        required: false
        type: string
        default: '3.12'
      install-groups:
        required: false
        type: string
        default: 'dev,test'

jobs:
  setup:
    runs-on: ubuntu-latest
    steps:
      - uses: actions/checkout@v5
      - uses: actions/setup-python@v4
        with:
          python-version: ${{ inputs.python-version }}
      - uses: snok/install-poetry@v1
        with:
          version: latest
          virtualenvs-create: true
          virtualenvs-in-project: true
      - uses: actions/cache@v4
        with:
          path: ~/.cache/pypoetry
          key: ${{ runner.os }}-poetry-${{ hashFiles('poetry.lock') }}
      - run: poetry install --with ${{ inputs.install-groups }}

Then call it:

jobs:
  setup:
    uses: ./.github/workflows/setup-python-env.yml
    with:
      install-groups: 'dev,test'

  tests:
    needs: setup
    runs-on: ubuntu-latest
    steps:
      # Dependencies already installed, just use them

Estimated savings: 2-3 minutes per PR

2. Poetry Lock Regeneration in CI (CRITICAL)

Problem: 05-ci.yml line 114 regenerates poetry.lock in CI

# Regenerate lock file to ensure sync
poetry lock

Impact:

• Disk Space: Poetry lock regeneration can exhaust GitHub runner disk space
• Time: Adds 30-60 seconds to every build
• Risk: Can cause CI failures on large dependency trees

Solution: Remove line 114. Lock file should ONLY be regenerated locally and committed.

# REMOVE THIS:
poetry lock

# CI should validate lock file is in sync (already done by poetry-lock-check.yml)
poetry check --lock  # This is enough

Root Cause: poetry-lock-check.yml already validates this. No need to regenerate.

Estimated savings: 30-60 seconds per run, prevents disk space failures

3. Redundant Disk Space Cleanup (MEDIUM PRIORITY)

Problem: Multiple workflows have identical disk cleanup code

Workflows affected:

• 01-lint.yml (lines 177-184)
• 03-build-secure.yml (lines 52-69)
• 04-pytest.yml (lines 45-54)

Current Duplication:

# Repeated in 3 workflows
- name: 🧹 Free Up Disk Space
  run: |
    sudo rm -rf /usr/share/dotnet
    sudo rm -rf /opt/ghc
    sudo rm -rf /usr/local/share/boost
    sudo rm -rf "$AGENT_TOOLSDIRECTORY"

Solution: Create composite action

# .github/actions/free-disk-space/action.yml
name: 'Free Disk Space'
description: 'Free up disk space on GitHub runners'
runs:
  using: "composite"
  steps:
    - shell: bash
      run: |
        echo "Initial: $(df -h / | awk 'NR==2 {print $4}') available"
        sudo rm -rf /usr/share/dotnet &
        sudo rm -rf /opt/ghc &
        sudo rm -rf /usr/local/share/boost &
        sudo rm -rf "$AGENT_TOOLSDIRECTORY" &
        wait
        echo "After cleanup: $(df -h / | awk 'NR==2 {print $4}') available"

Then use it:

- uses: ./.github/actions/free-disk-space

Estimated savings: Cleaner code, easier maintenance

4. Lint Workflow - Inefficient Poetry Installs (HIGH PRIORITY)

Problem: 01-lint.yml installs poetry 4 separate times in matrix (lines 129, 141, 149, 157)

Current:

- id: ruff
  cmd: |
    pip install poetry
    poetry install --only dev
    poetry run ruff check ...

- id: mypy
  cmd: |
    pip install poetry  # DUPLICATE
    poetry install --only dev  # DUPLICATE
    poetry run mypy ...

Solution: Install once, use for all matrix jobs

steps:
  - uses: actions/checkout@v5

  - uses: actions/setup-python@v4
    if: needs-python-tools
    with:
      python-version: '3.12'

  - name: Install Poetry Once
    if: needs-python-tools
    run: pip install poetry

  - name: Install Dependencies Once
    if: needs-python-tools
    run: poetry install --only dev

  - name: Run Matrix Command
    run: ${{ matrix.cmd }}

Estimated savings: 1-2 minutes per lint run

5. Test Environment Variables Duplication (LOW PRIORITY)

Problem: Same env vars defined in multiple workflows

Workflows affected:

• 04-pytest.yml (lines 22-37)
• 05-ci.yml (lines 72-85)

Solution: Create shared env file or use GitHub Environment

# Option 1: Shared composite action
# .github/actions/setup-test-env/action.yml

# Option 2: Use organization/repo environment
# Settings → Environments → test-env
# Then reference in workflow:
jobs:
  test:
    environment: test-env  # Automatically loads all env vars

Estimated savings: Easier maintenance, single source of truth

6. 05-ci.yml - Redundant Steps (MEDIUM PRIORITY)

Problem: Lines 118-128 run linting/security that's already in 01-lint.yml and 02-security.yml

- name: Check formatting
  run: make format-check
  continue-on-error: true

- name: Run linting (Ruff + MyPy)
  run: make lint
  continue-on-error: true

- name: Run security checks
  run: make security-check
  continue-on-error: true

Solution: Remove these. They're already run by dedicated workflows.

Rationale:

• 05-ci.yml only runs on push to main (after PR merge)
• 01-lint.yml and 02-security.yml already ran during PR
• No value in re-running with continue-on-error: true

Estimated savings: 30-60 seconds per main push

7. Makefile Testing - Docker Build Test (RECENTLY FIXED)

Status: ✅ FIXED in commit 2184b9c

What was fixed: Poetry files copied from wrong location after root migration

Impact: No longer an issue

Recommended Optimizations

Priority 1: Critical Fixes (Immediate)

1. Remove poetry lock regeneration in 05-ci.yml:114
   • Impact: Prevents disk space failures
   • Effort: 1 minute
   • Savings: 30-60s per run + prevents CI failures

Priority 2: High-Impact Optimizations (This Week)

2. Consolidate Poetry installation in 01-lint.yml

   • Impact: 4 redundant installs → 1 install
   • Effort: 15 minutes
   • Savings: 1-2 minutes per lint run

3. Create reusable Python setup workflow

   • Impact: Used by 4+ workflows
   • Effort: 30 minutes
   • Savings: 2-3 minutes per PR

4. Remove redundant linting from 05-ci.yml

   • Impact: Duplicate checks
   • Effort: 5 minutes
   • Savings: 30-60s per main push

Priority 3: Code Quality (This Sprint)

5. Create disk cleanup composite action

   • Impact: DRY principle, easier maintenance
   • Effort: 20 minutes
   • Savings: Cleaner code

6. Centralize test environment variables

   • Impact: Single source of truth
   • Effort: 15 minutes
   • Savings: Easier maintenance

Estimated Total Impact

Optimization	Time Saved Per PR	Reliability Improvement
Remove poetry lock regen	30-60s	Prevents disk failures
Consolidate Poetry installs	1-2 min	-
Reusable Python setup	2-3 min	-
Remove redundant linting	30-60s	-
TOTAL	4-6 minutes	High

Current PR Iteration Time Analysis

From Issue #507 (PR #506 example):

• Current: 6-8 iterations to green CI
• Industry Standard: 2-4 iterations
• Target: ~3 commits to green CI

Root Causes:

1. Unseen State: CI environment differs from local
2. Cascade Effect: Each fix reveals new issue
3. No Pre-flight Checks: Issues discovered one at a time

CI/CD Contribution to Iteration Time:

• Slow workflows: 5-10 min per iteration
• Failed fast checks: Restart entire workflow
• Duplicate checks: Wasted compute time

Additional Recommendations

A. Add Job Dependencies for Fail-Fast

jobs:
  quick-checks:
    # Fast checks (30s): YAML, JSON, merge conflicts

  lint:
    needs: quick-checks
    # Medium checks (2 min): Ruff, type checking

  tests:
    needs: lint
    # Slow checks (5 min): Unit tests

Benefit: Fail fast on syntax errors before running expensive tests

B. Optimize Cache Strategy

Current caching is good, but can be improved:

# Current
path: ~/.cache/pypoetry

# Optimized - cache venv too
path: |
  ~/.cache/pypoetry
  .venv

C. Add Workflow Status Dashboard

Create .github/workflows/status-dashboard.yml:

# Runs after all PR checks complete
# Generates single summary of all workflow results
# Easier to see what failed without clicking each job

Implementation Plan

Week 1: Critical Fixes

• Remove poetry lock regeneration (05-ci.yml)
• Consolidate Poetry installs (01-lint.yml)
• Test changes on feature branch

Week 2: High-Impact Optimizations

• Create reusable Python setup workflow
• Create disk cleanup composite action
• Update workflows to use new reusable components
• Remove redundant checks from 05-ci.yml

Week 3: Polish & Documentation

• Centralize test environment variables
• Add workflow status dashboard
• Document optimization strategy
• Update CLAUDE.md with CI best practices

Success Metrics

Before Optimization:

• Average PR iterations: 6-8
• Average time per iteration: 5-10 min
• Total time to green: 30-80 min

After Optimization (Target):

• Average PR iterations: 2-3
• Average time per iteration: 3-5 min
• Total time to green: 6-15 min

Reduction: ~70% faster (24-65 min saved per PR)

Related Issues

• Reduce PR Iteration Time: Too Many CI Failures Before Success #507 - Reduce PR Iteration Time (this analysis)
• Refactor E2E Test Targets for Clarity and Isolation #505 - E2E Test Refactoring
• Streamline Makefile: Remove outdated targets and adopt IBM MCP Context Forge best practices #348 - Makefile Simplification
• refactor: Move Poetry configuration to project root for cleaner monorepo structure #501 - Poetry Root Migration (recently completed)

Questions for Discussion

1. Should we implement all optimizations at once or incrementally?
2. Is there value in keeping redundant checks in 05-ci.yml?
3. Should we create organization-level reusable workflows for other projects?
4. Do we want workflow status dashboard or is GitHub UI sufficient?

---

Priority: High
Effort: 2-3 hours total (spread across 3 weeks)
Impact: High - 70% faster PR feedback, improved developer experience