diff --git a/.github/workflows/carto-feature-tests.yml b/.github/workflows/carto-feature-tests.yml new file mode 100644 index 00000000000..0d5146f7f1a --- /dev/null +++ b/.github/workflows/carto-feature-tests.yml @@ -0,0 +1,124 @@ +name: CARTO Feature Tests + +# Runs the unit-test suites that cover CARTO's customized modules on every PR +# to carto/main. Upstream's own test workflows (test-unit-*.yml, test-linting) +# only trigger for upstream branches (main, litellm_**), so without this gate +# no Python test executes against carto/main PRs at all. That gap let the +# PR #121 sync ship a broken handler->iterator wiring that only surfaced in +# cloud-native integration tests, 3 repos downstream. +# +# The test scope is derived from .github/carto-features.yml: each feature +# file's mirrored directory under tests/test_litellm/ (walking up to the +# nearest existing dir). Directory-level mirroring is deliberate - it pulls in +# tests for auto-merged sibling files (e.g. handler.py) whose wiring can break +# when a conflicted neighbor is resolved against them. +# +# The ready-checker requires every check on an upstream-sync PR to pass before +# adding sync-ready, so a failure here also blocks the automated release chain. + +on: + pull_request: + branches: + - carto/main + paths: + - "litellm/**" + - "litellm-proxy-extras/**" + - "tests/**" + - "pyproject.toml" + - "uv.lock" + - ".github/carto-features.yml" + - ".github/workflows/carto-feature-tests.yml" + +permissions: + contents: read + +concurrency: + group: ${{ github.workflow }}-${{ github.event.pull_request.number || github.ref }} + cancel-in-progress: true + +jobs: + feature-tests: + name: CARTO feature unit tests + runs-on: ubuntu-latest + timeout-minutes: 40 + + steps: + - uses: actions/checkout@08eba0b27e820071cde6df949e0beb9ba4906955 # v4.3.0 + with: + persist-credentials: false + + - name: Set up Python + uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5.6.0 + with: + python-version: "3.12" + + - name: Set up uv + uses: astral-sh/setup-uv@37802adc94f370d6bfd71619e3f0bf239e1f3b78 # v7 + with: + version: "0.10.9" + + - name: Cache uv dependencies + uses: actions/cache@0057852bfaa89a56745cba8c7296529d2fc39830 # v4.3.0 + with: + path: | + ~/.cache/uv + .venv + key: ${{ runner.os }}-uv-feature-tests-${{ hashFiles('uv.lock') }} + restore-keys: | + ${{ runner.os }}-uv-feature-tests- + + - name: Install test dependencies + run: make install-test-deps + + - name: Import smoke check + run: | + uv run --no-sync python -c "import litellm" \ + || (echo "::error::'import litellm' failed - broken imports on this branch" && exit 1) + + - name: Derive test scope from CARTO features manifest + id: scope + run: | + set -eu + # uv venv python: PyYAML is a litellm dependency; the runner's bare python3 lacks it + PATHS=$(uv run --no-sync python - <<'EOF' + import pathlib + import yaml + + with open(".github/carto-features.yml") as f: + data = yaml.safe_load(f) + + dirs = set() + for feat in data.get("features", []): + for fp in feat.get("files", []): + p = pathlib.PurePosixPath(fp) + if p.parts[0] != "litellm": + continue + rel = pathlib.PurePosixPath(*p.parts[1:]).parent + while True: + cand = pathlib.Path("tests/test_litellm") / rel + if cand.is_dir(): + dirs.add(str(cand)) + break + if rel == pathlib.PurePosixPath("."): + break + rel = rel.parent + + print(" ".join(sorted(dirs))) + EOF + ) + if [ -z "$PATHS" ]; then + echo "::error::No test directories derived from .github/carto-features.yml" + exit 1 + fi + echo "Derived test scope:" + echo "$PATHS" | tr ' ' '\n' + echo "paths=$PATHS" >> $GITHUB_OUTPUT + + - name: Run CARTO feature tests + run: | + uv run --no-sync pytest ${{ steps.scope.outputs.paths }} \ + --tb=short -q \ + -n 2 \ + --reruns 2 \ + --reruns-delay 1 \ + --durations=15 diff --git a/.github/workflows/carto-upstream-sync-resolver.yml b/.github/workflows/carto-upstream-sync-resolver.yml index 3f2e17c6701..4a86b13eb20 100644 --- a/.github/workflows/carto-upstream-sync-resolver.yml +++ b/.github/workflows/carto-upstream-sync-resolver.yml @@ -824,6 +824,34 @@ jobs: --- + ## CROSS-FILE WIRING CHECK (CRITICAL) + + Git auto-merges files that only one side changed - those files are NOT in your + conflict list, but they can still call code you are rewriting. This caused a + production outage in PR #121: streaming_iterator.py (conflicted) lost a CARTO + __init__ parameter while handler.py (auto-merged, same package) still passed it, + so every streaming request failed with TypeError. + + After resolving each conflicted Python file, verify its wiring against the whole + package, not just the conflicted set: + + 1. For every function/method whose SIGNATURE you changed or replaced with the + upstream version, grep the repo for its call sites and confirm every kwarg + they pass still exists in the signature: + ```bash + grep -rn "(" litellm/ | grep -v "def " + ``` + 2. For every `self.` read inside methods you kept (especially CARTO patch + methods like _store_session_in_redis), confirm the attribute is assigned in + __init__ or before use. + 3. If a CARTO feature spans multiple files (check the manifest `files:` lists), + re-read ALL of that feature's files together after resolution - including the + ones git auto-merged - and confirm params, attributes, and imports line up. + + A feature whose strings grep OK but whose wiring is broken is NOT preserved. + + --- + ## SYNC-REQUIRED FILES (Always Accept Upstream TAG) These files MUST match upstream regardless of CARTO history: @@ -1160,8 +1188,12 @@ jobs: exit 0 fi + # Blocking check: Claude updates the manifest itself when upstream legitimately + # substitutes a feature (as in PR #121), so a pattern still missing at this + # point means the feature was dropped without accounting for it. Failing here + # blocks the sync-ready label instead of surfacing 3 repos downstream. python3 -c " - import yaml, subprocess + import yaml, subprocess, sys with open('.github/carto-features.yml') as f: data = yaml.safe_load(f) missing = [] @@ -1174,11 +1206,12 @@ jobs: print(f'MISSING: {feat[\"name\"]} - {v[\"pattern\"]} in {v[\"file\"]}') missing.append(f'{feat[\"name\"]}: {v[\"pattern\"]} in {v[\"file\"]}') if missing: - print(f'\n::warning::CARTO features manifest: {len(missing)} pattern(s) missing (Claude may have intentionally changed code):') + print(f'\n::error::CARTO features manifest: {len(missing)} pattern(s) missing after resolution:') for m in missing: print(f' - {m}') - else: - print('\nAll CARTO feature patterns verified.') + print('If upstream legitimately substituted a feature, the manifest must be updated in the same resolution.') + sys.exit(1) + print('\nAll CARTO feature patterns verified.') " echo "::endgroup::"