Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
49 changes: 49 additions & 0 deletions .github/workflows/CARTO_UPSTREAM_SYNC.md
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,55 @@ The workflow sends Slack notifications to `#cartodb-ops`:
3. Run locally: `make lint && make test-unit`
4. Push fixes to the sync branch

### A CARTO feature broke after a sync (silent wiring loss)

The v1.92.0 sync is the case study: the merge left CARTO customizations
that PASSED every presence check yet were functionally broken. Three
distinct wiring failures, each invisible to string-grep verification, only
surfaced in cloud-native integration tests three repos downstream. When a
feature misbehaves after a sync but its manifest patterns still grep OK,
check these in order.

**1. Dropped call site across an auto-merged file.** Git auto-merges files
only one side changed; they are not in the resolver's conflict list, so a
signature rewritten in a conflicted file can leave a caller in an
auto-merged sibling passing a now-removed argument. Symptom: `TypeError:
... got an unexpected keyword argument`. In v1.92.0, `streaming_iterator.py`
(conflicted) lost a param while `handler.py` (auto-merged) kept passing it.
Find it by grepping call sites of any rewritten signature:
```bash
grep -rn "LiteLLMCompletionStreamingIterator(" litellm/ | grep -v "def "
```

**2. Orphaned CARTO helper (present but never called).** A helper survives
the merge byte-for-byte, so its `def` pattern greps OK, but the code that
CALLED it was replaced by the upstream version. Symptom: the feature simply
does nothing. In v1.92.0, `_patch_get_session_from_redis` was defined but
had zero callers, so sessions were written to Redis but read from the
batch-delayed DB, and multi-turn conversations lost context. Find orphans:
```bash
for fn in $(git grep -hoE "def (_patch_[a-z_0-9]+|_carto_[a-z_0-9]+)" -- litellm/ | sed -E 's/def //' | sort -u); do
refs=$(git grep -c "$fn" -- litellm/ | awk -F: '{s+=$NF} END {print s}')
defs=$(git grep -c "def $fn" -- litellm/ | awk -F: '{s+=$NF} END {print s}')
[ "$refs" -le "$defs" ] && echo "ORPHAN: $fn"
done
```

**3. Cross-version data-format drift.** CARTO code is unchanged and fully
wired, but upstream changed the format of a value flowing through it, so a
stored key no longer matches its lookup. Symptom: silent 100% cache/lookup
miss. In v1.92.0, upstream began b64-encoding response ids; CARTO's Redis
store keyed by the encoded id while the lookup used the decoded id. This
class cannot be found by grep - only by tracing what each feature consumes
and produces across the version boundary, or by a behavioral test.

**Fix approach for all three:** restore the CARTO block verbatim from
`origin/carto/main` (never paraphrase); adapt only the call site or the
data-format handling, minimally, marked `# CARTO PATCH`. The regression
canaries in `tests/test_litellm/responses/litellm_completion_transformation/`
pin these three wirings; the CARTO Feature Tests gate runs them on every
sync PR, and the CI fixer reacts to that gate's failures.

### Workflow Not Detecting New Releases

1. Check `gh release list --repo BerriAI/litellm` for the latest non-prerelease, non-draft tag (BerriAI dropped the `-stable` suffix after `v1.83.14-stable`, published 2026-05-02 — releases since are plain `vX.Y.Z` tags)
Expand Down
98 changes: 83 additions & 15 deletions .github/workflows/carto-upstream-sync-ci-fixer.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,15 @@ name: CARTO Upstream Sync - CI Fixer

on:
workflow_run:
# Only workflows that actually run against carto/main PRs. "LiteLLM Mock
# Tests" is upstream-deprecated (workflow_dispatch only) and "LiteLLM
# Linting" only triggers for upstream's own branches, never carto/main -
# both were removed. "CARTO Feature Tests" is CARTO's own unit-test gate
# (carto-feature-tests.yml); listing it here is what lets the fixer react
# to a failing test suite the way it used to react to Mock Tests.
workflows:
- "LiteLLM Mock Tests (folder - tests/test_litellm)"
- "CARTO - Deploy Docker Image (CI)"
- "LiteLLM Linting"
- "CARTO Feature Tests"
types:
- completed
branches:
Expand Down Expand Up @@ -207,11 +212,14 @@ jobs:
touch /tmp/docker_full_log.txt
fi

# Extract Mock Tests errors - get the FULL failed job log
# Extract CARTO Feature Tests errors - get the FULL failed job log.
# This is CARTO's own unit-test gate (carto-feature-tests.yml); it
# replaced the upstream-deprecated "LiteLLM Mock Tests" workflow as
# the source of test-failure signal on carto/main PRs.
echo "" >> /tmp/all_errors.txt
echo "## Mock Tests Errors" >> /tmp/all_errors.txt
echo "## CARTO Feature Tests Errors" >> /tmp/all_errors.txt
TESTS_RUN=$(gh run list --repo ${{ github.repository }} \
--workflow="LiteLLM Mock Tests (folder - tests/test_litellm)" \
--workflow="CARTO Feature Tests" \
--branch="${BRANCH}" \
--status=failure \
--json databaseId \
Expand All @@ -223,10 +231,10 @@ jobs:
gh run view $TESTS_RUN --repo ${{ github.repository }} --log-failed 2>&1 > /tmp/tests_full_log.txt
TESTS_LINES=$(wc -l < /tmp/tests_full_log.txt)
echo "[CI Fixer] Tests log: ${TESTS_LINES} lines"
echo "Mock Tests Run ID: $TESTS_RUN (${TESTS_LINES} lines)" >> /tmp/all_errors.txt
echo "CARTO Feature Tests Run ID: $TESTS_RUN (${TESTS_LINES} lines)" >> /tmp/all_errors.txt
echo "Full log saved to: /tmp/tests_full_log.txt" >> /tmp/all_errors.txt
else
echo "No failed Mock Tests found" >> /tmp/all_errors.txt
echo "No failed CARTO Feature Tests found" >> /tmp/all_errors.txt
touch /tmp/tests_full_log.txt
fi

Expand Down Expand Up @@ -414,10 +422,14 @@ jobs:
**PR:** #${{ needs.check-ci-status.outputs.pr-number }}
**Branch:** `${{ needs.check-ci-status.outputs.branch-name }}`
**Docker Build Failed:** ${{ steps.extract-errors.outputs.docker-failed }}
**Mock Tests Failed:** ${{ steps.extract-errors.outputs.tests-failed }}
**CARTO Feature Tests Failed:** ${{ steps.extract-errors.outputs.tests-failed }}

**Key insight:** The upstream TAG code is TESTED and WORKING. If something is "missing",
the conflict resolver likely kept an old carto/main version instead of the new upstream TAG.
**Key insight:** Upstream TAG code works for UPSTREAM's call graph, not
necessarily for CARTO's. It is tested, but CARTO adds call sites,
parameters, and stored-data contracts upstream never exercises. When a
fix accepts an upstream file, re-verify CARTO's callers, attributes, and
data formats still line up - a file that imports cleanly can still be
broken at every CARTO call site.

---

Expand All @@ -431,6 +443,25 @@ jobs:
These patterns MUST still exist after your fixes. If a fix would remove a
pattern, find an alternative approach that preserves the CARTO feature.

**Restore CARTO code VERBATIM, do not paraphrase.** When a fix needs a
CARTO block back, copy it byte-identical from carto/main
(`git show origin/carto/main:<file>`) - including comments and debug
logging - and confirm with `diff`. Adapt only where an upstream API
change makes verbatim impossible, keep it minimal, and mark it
`# CARTO PATCH`.

**The manifest grep proves a string EXISTS, not that it is WIRED.**
After your fix, run an orphan sweep - a CARTO helper defined with no
caller is a half-restored feature (the v1.92.0 sync left
`_patch_get_session_from_redis` defined but uncalled):
```bash
for fn in $(git grep -hoE "def (_patch_[a-z_0-9]+|_carto_[a-z_0-9]+)" -- litellm/ | sed -E 's/def //' | sort -u); do
refs=$(git grep -c "$fn" -- litellm/ | awk -F: '{s+=$NF} END {print s}')
defs=$(git grep -c "def $fn" -- litellm/ | awk -F: '{s+=$NF} END {print s}')
[ "$refs" -le "$defs" ] && echo "ORPHAN: $fn (defined, never called)"
done
```

---

## STEP 0: FIX LOOP DETECTION (DO THIS FIRST!)
Expand All @@ -441,13 +472,26 @@ jobs:
cat /tmp/pr_comments.md | grep -iE "Added.*function|sync.*file|ImportError|fix:" | head -15
```

**If same file appears 3+ times → SYNC ENTIRE FILE from upstream TAG:**
**If same file appears 3+ times → break the loop, BUT check the
manifest first.**

If the file is NOT in any feature's `files:` in
`.github/carto-features.yml`, sync the entire file from upstream TAG:
```bash
# ORIG_HEAD = upstream TAG version (pre-merge state)
git show ORIG_HEAD:path/to/problematic_file.py > path/to/problematic_file.py
git add path/to/problematic_file.py
```

If the file IS a manifest file, do NOT blindly sync it from upstream -
that is exactly how CARTO wirings get erased. Instead, take upstream's
version as the base and re-apply the CARTO block VERBATIM from
carto/main, then run the orphan/wiring checks:
```bash
git show ORIG_HEAD:<file> > <file> # upstream base
git show origin/carto/main:<file> | less # copy CARTO blocks back byte-identical
```

---

## STEP 1: READ CI LOGS
Expand All @@ -466,7 +510,7 @@ jobs:
# Read test errors (if tests failed)
if [ -s /tmp/tests_full_log.txt ]; then
echo ""
echo "=== MOCK TEST ERRORS (filtered) ==="
echo "=== CARTO FEATURE TEST ERRORS (filtered) ==="
grep -iE "FAILED|ERROR|AssertionError|ImportError|Exception" /tmp/tests_full_log.txt | tail -30
echo ""
echo "=== TEST LOG (last 100 lines for context) ==="
Expand All @@ -490,10 +534,34 @@ jobs:
cd ui/litellm-dashboard && npm install --legacy-peer-deps && npm run build 2>&1 | tail -50
```

### If Mock Tests Failed:
### If CARTO Feature Tests Failed:
Reproduce the exact gate locally. The scope is the set of test
directories that mirror the files in `.github/carto-features.yml`
(the same derivation carto-feature-tests.yml uses), so a fix that
passes here passes the gate.
```bash
pip install -e ".[dev]" -q
pytest tests/test_litellm/ -v --tb=short 2>&1 | tail -100
make install-test-deps
# Derive the same scope the gate runs, then execute it:
SCOPE=$(uv run --no-sync python - <<'PY'
import pathlib, yaml
data = yaml.safe_load(open(".github/carto-features.yml"))
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)))
PY
)
uv run --no-sync pytest $SCOPE -v --tb=short 2>&1 | tail -100
```

**Iterate:** Fix error → Run verification → Repeat until success.
Expand Down
22 changes: 22 additions & 0 deletions .github/workflows/carto-upstream-sync-customizations-analyzer.yml
Original file line number Diff line number Diff line change
Expand Up @@ -446,6 +446,28 @@ jobs:
| PRESERVED_CARTO | Full CARTO implementation kept | CARTO code present, upstream version not used |
| INCORRECTLY_DROPPED | CARTO feature was lost (BUG!) | CARTO code missing, upstream doesn't provide equivalent |

## Judge by WIRING, not string presence

A feature is PRESERVED_CARTO only if it is fully WIRED, not merely
present. Code that greps OK can still be dead. Before classifying any
feature as preserved, verify:

- **No orphaned helpers:** every CARTO helper the feature defines
(`_patch_*` / `_carto_*`) has at least one caller. A helper defined
with zero call sites means the feature is INCORRECTLY_DROPPED, even
though its `def` string is present. (v1.92.0 left
`_patch_get_session_from_redis` defined but uncalled - a real bug the
string-presence check reported as "preserved".)
- **Call sites intact across auto-merged files:** parameters the
feature relies on are still passed by callers (which may live in
files git auto-merged, not just the conflicted ones).
- **Data-flow contracts hold:** values the feature stores/sends still
match the format the surrounding upstream code now expects (e.g. id
encoding, message shape).

If any of these fail, the correct decision is INCORRECTLY_DROPPED with
the specific broken wiring named in the evidence.

CONTEXT_EOF

# Add CARTO features list
Expand Down
11 changes: 9 additions & 2 deletions .github/workflows/carto-upstream-sync-ready-checker.yml
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,17 @@ name: CARTO Upstream Sync - Ready Checker

on:
workflow_run:
# Only workflows that actually run against carto/main PRs. "LiteLLM Mock
# Tests" is upstream-deprecated (workflow_dispatch only) and "LiteLLM
# Linting" only triggers for upstream's own branches, never carto/main -
# both were removed. "CARTO Feature Tests" is CARTO's own unit-test gate
# (carto-feature-tests.yml); it must be listed here so sync-ready waits for
# the tests, not just the Docker build. workflow_run only re-invokes this
# workflow when a LISTED workflow completes, so an unlisted test gate that
# finishes after the Docker build would never get sync-ready applied.
workflows:
- "LiteLLM Mock Tests (folder - tests/test_litellm)"
- "CARTO - Deploy Docker Image (CI)"
- "LiteLLM Linting"
- "CARTO Feature Tests"
types:
- completed
branches:
Expand Down
Loading
Loading