Skip to content

docs(ci): align sync resolver/fixer/analyzer prompts with wiring-aware reasoning #126

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mateo-di merged 1 commit into from
Aug 4, 2026
Merged

docs(ci): align sync resolver/fixer/analyzer prompts with wiring-aware reasoning #126

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
View file
Open in desktop
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
42 changes: 39 additions & 3 deletions .github/workflows/carto-upstream-sync-ci-fixer.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -424,8 +424,12 @@ jobs:
**Docker Build Failed:** ${{ steps.extract-errors.outputs.docker-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 @@ -439,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 @@ -449,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 Down
22 changes: 22 additions & 0 deletions .github/workflows/carto-upstream-sync-customizations-analyzer.yml
View file
Open in desktop
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
94 changes: 90 additions & 4 deletions .github/workflows/carto-upstream-sync-resolver.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -770,7 +770,27 @@ jobs:
## CARTO FEATURES TO PRESERVE

These are the INTENTIONAL CARTO customizations from merged PRs.
**Preserve the BEHAVIOR, not necessarily the exact file versions.**

**VERBATIM-FIRST RULE (do not paraphrase CARTO code).** For any file
listed in a feature's `files:` in `.github/carto-features.yml`,
restore the CARTO block BYTE-IDENTICAL from carto/main:
```bash
git show origin/carto/main:<file> # source of truth for CARTO blocks
```
Copy the exact lines (including comments and debug logging) and
confirm with `diff`. Do NOT rewrite from memory or "clean it up" -
a paraphrase that looks equivalent is how wiring silently breaks
(v1.92.0 dropped a session read-path this way while its helper
survived).

Adaptation is allowed ONLY when an upstream API change makes a
verbatim copy impossible (e.g. a signature or data-format change).
When it is, keep the change minimal (a few lines), mark it
`# CARTO PATCH` with the reason, and prefer adapting the CALL SITE
over rewriting the CARTO block. Worked example of a legitimate
adaptation: v1.92.0 began b64-encoding response ids, so the Redis
session store had to decode `response.id` before keying - the CARTO
helper stayed byte-identical, only the 2 call sites changed.

Read the feature context:
```bash
Expand Down Expand Up @@ -822,6 +842,66 @@ jobs:

If ANY patterns are MISSING, restore them from carto/main or re-implement.

**The manifest grep proves a string EXISTS, not that it is WIRED.**
A helper can exist with no caller; a parameter can be accepted but
never passed; a stored key can no longer match its lookup. The three
checks below catch what the grep cannot. Do all three before
completing the merge.

### CHECK 1: Orphan sweep (dropped call sites)
For every CARTO helper (functions named `_patch_*` / `_carto_*` and
anything marked `# CARTO`), confirm it still has a caller:
```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
```
An orphaned CARTO helper means its feature is only half-restored (the
v1.92.0 sync left `_patch_get_session_from_redis` defined but uncalled,
so sessions were written to Redis but read from the batch-delayed DB).
Restore the call site verbatim from carto/main.

### CHECK 2: Cross-file wiring (auto-merged siblings)
Git auto-merges files only one side changed - they are NOT in your
conflict list, but they can call code you just rewrote. After
resolving each conflicted file, for every signature you changed or
replaced with upstream's, grep the whole repo for its call sites and
confirm every kwarg still exists:
```bash
grep -rn "<function_or_class_name>(" litellm/ | grep -v "def "
```
(v1.92.0: `streaming_iterator.py` was conflicted and lost a param
while the auto-merged `handler.py` kept passing it - every streaming
request then raised TypeError.) For any multi-file manifest feature,
re-read ALL of its `files:` together, including auto-merged ones.

### CHECK 3: Cross-version data-flow
For each feature, trace what it CONSUMES and PRODUCES through the new
upstream code - ids, message shapes, metadata keys - not just whether
its text survived. Ask: does the value this CARTO code stores/sends
still match the format the (possibly rewritten) upstream code around
it now expects? (v1.92.0 began b64-encoding response ids: CARTO's
unchanged store keyed Redis by the encoded id while the lookup used
the decoded id, so every session lookup missed.)

### Canary tests (the definition of "wired correctly")
These regression tests encode the wirings that broke in the v1.92.0
sync. The CARTO Feature Tests gate runs them on the PR; your
resolution must keep them green. If a test environment is available,
run them before completing the merge:
```bash
uv run --no-sync pytest \
tests/test_litellm/responses/litellm_completion_transformation/test_session_handler.py \
tests/test_litellm/responses/litellm_completion_transformation/test_tool_call_streaming_transformation.py \
-q 2>&1 | tail -20
```
They pin: the streaming iterator's `litellm_completion_request`
wiring, the Redis-first session read, and the store-key ==
decoded-lookup-key roundtrip. A green grep with a red canary means
the feature is broken - fix the wiring, do not touch the test.

---

## SYNC-REQUIRED FILES (Always Accept Upstream TAG)
Expand Down Expand Up @@ -864,10 +944,16 @@ jobs:

## KEY PRINCIPLE

**Upstream TAG code WORKS** - it's from a tested stable release. If something is "missing",
the conflict resolver probably kept an old carto/main version instead of the new upstream TAG.
**Upstream TAG code works for UPSTREAM's call graph - not necessarily
for CARTO's.** It is from a tested stable release, but CARTO adds call
sites, parameters, and stored-data contracts that upstream's tests
never exercise. So when you accept an upstream file, you are NOT done:
you must re-verify that CARTO's callers, attributes, and data formats
still line up (see the wiring and data-flow checks above). A file that
imports cleanly can still be broken at every CARTO call site.

When in doubt: Compare file sizes. Bigger = usually newer = probably correct.
Do NOT choose versions by file size or "bigger = newer". Decide by what
preserves CARTO behavior verbatim while adopting upstream's changes.

---

Expand Down