docs(ci): align sync resolver/fixer/analyzer prompts with wiring-aware reasoning - #126
Merged
mateo-di merged 1 commit intoAug 4, 2026
Conversation
…e reasoning The v1.92.0 sync shipped three CARTO features that passed every automated check yet were functionally broken (dropped call site across an auto-merged file, an orphaned helper with no caller, and a store/lookup key that mismatched after upstream changed id encoding). None were catchable by the prompts' existing "does the pattern still grep" verification, and two of the prompts' heuristics actively caused the loss. This aligns the prompts with the reasoning that actually found and fixed those bugs. Resolver prompt: - Verbatim-first rule: restore CARTO blocks byte-identical from carto/main (diff-verified); adapt only where an upstream API change makes verbatim impossible, minimally and marked # CARTO PATCH. Replaces "preserve the BEHAVIOR, not necessarily the exact file versions", which licensed the paraphrase that dropped a session read-path. - Remove harmful heuristics: delete "bigger file = probably correct" and reframe "upstream TAG code WORKS" as "works for upstream's call graph, not necessarily CARTO's - re-verify callers, attributes, data formats". - Add three post-resolution checks the grep cannot do: orphan sweep (helper with no caller), cross-file wiring (auto-merged siblings), and cross-version data-flow (format drift across the version boundary). - Reference the regression canaries as the definition of "wired correctly". Fixer prompt: - Same verbatim-first rule, orphan sweep, and call-graph reframing. - Manifest-aware loop guard: the "3+ fixes -> sync entire file from upstream" escape hatch now excludes manifest files; for those, take upstream as base and re-apply the CARTO block verbatim (blind sync is how wirings get erased). - Log extraction / verification already retargeted to CARTO Feature Tests in the agent-reaction PR this is stacked on. Analyzer prompt: - Judge PRESERVED_CARTO by WIRING, not string presence: an orphaned helper, a missing call site, or broken data-flow is INCORRECTLY_DROPPED even when the def/pattern greps OK. It had reported the dead session read as PASS. CARTO_UPSTREAM_SYNC.md: - New troubleshooting section documenting the three v1.92.0 wiring-loss classes with detection commands. Manifest wiring-pattern enrichment (adding call-site patterns to carto-features.yml) is deliberately deferred to a follow-up to avoid clobbering the manifest changes on the open v1.92.0 sync PR.
mateo-di
marked this pull request as ready for review
August 4, 2026 12:22
mateo-di
added a commit
that referenced
this pull request
Aug 4, 2026
…sts (#125) * fix(ci): restore agent reaction to test failures via CARTO Feature Tests The upstream-sync fixer and ready-checker were wired to react to "LiteLLM Mock Tests" and "LiteLLM Linting" workflow_run completions. Both are dead on carto/main: Mock Tests is upstream-deprecated (workflow_dispatch only) and Linting only triggers for upstream's own branches. So no test ever ran on a carto/main sync PR, the fixer never fired, and the only failure signal was cloud-native integration tests three repos downstream (see the v1.92.0 sync, where three broken CARTO wirings shipped that way). Wire both workflows to "CARTO Feature Tests" (carto-feature-tests.yml, CARTO's own unit-test gate) instead: - ready-checker: replace the two dead names with Docker CI + Feature Tests. workflow_run only re-invokes ready-checker when a listed workflow completes, so listing Feature Tests is what makes sync-ready wait for the tests rather than just the build. - ci-fixer: same trigger list; swap the log-extraction query from the dead Mock Tests workflow to Feature Tests; align the prompt's local verification with the gate's real command (uv + make install-test-deps + manifest-derived scope) instead of the stale pip/pytest invocation. Supersedes #123 (which only removed the dead names). Depends on the CARTO Feature Tests workflow from #124. * docs(ci): align sync resolver/fixer/analyzer prompts with wiring-aware reasoning (#126) The v1.92.0 sync shipped three CARTO features that passed every automated check yet were functionally broken (dropped call site across an auto-merged file, an orphaned helper with no caller, and a store/lookup key that mismatched after upstream changed id encoding). None were catchable by the prompts' existing "does the pattern still grep" verification, and two of the prompts' heuristics actively caused the loss. This aligns the prompts with the reasoning that actually found and fixed those bugs. Resolver prompt: - Verbatim-first rule: restore CARTO blocks byte-identical from carto/main (diff-verified); adapt only where an upstream API change makes verbatim impossible, minimally and marked # CARTO PATCH. Replaces "preserve the BEHAVIOR, not necessarily the exact file versions", which licensed the paraphrase that dropped a session read-path. - Remove harmful heuristics: delete "bigger file = probably correct" and reframe "upstream TAG code WORKS" as "works for upstream's call graph, not necessarily CARTO's - re-verify callers, attributes, data formats". - Add three post-resolution checks the grep cannot do: orphan sweep (helper with no caller), cross-file wiring (auto-merged siblings), and cross-version data-flow (format drift across the version boundary). - Reference the regression canaries as the definition of "wired correctly". Fixer prompt: - Same verbatim-first rule, orphan sweep, and call-graph reframing. - Manifest-aware loop guard: the "3+ fixes -> sync entire file from upstream" escape hatch now excludes manifest files; for those, take upstream as base and re-apply the CARTO block verbatim (blind sync is how wirings get erased). - Log extraction / verification already retargeted to CARTO Feature Tests in the agent-reaction PR this is stacked on. Analyzer prompt: - Judge PRESERVED_CARTO by WIRING, not string presence: an orphaned helper, a missing call site, or broken data-flow is INCORRECTLY_DROPPED even when the def/pattern greps OK. It had reported the dead session read as PASS. CARTO_UPSTREAM_SYNC.md: - New troubleshooting section documenting the three v1.92.0 wiring-loss classes with detection commands. Manifest wiring-pattern enrichment (adding call-site patterns to carto-features.yml) is deliberately deferred to a follow-up to avoid clobbering the manifest changes on the open v1.92.0 sync PR.
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Aligns the upstream-sync automation prompts with the reasoning that actually found and fixed the v1.92.0 regressions. That sync shipped three CARTO features which passed every automated check yet were functionally broken, and two of the prompts' own heuristics caused the loss. This is the prompt/doc half of the long-term hardening (Workplan B); the mechanical test-reaction wiring is #125 (Workplan A), which this is stacked on.
The three failure classes, none catchable by "does the pattern still grep":
streaming_iterator.py(conflicted) lost a param whilehandler.py(auto-merged) kept passing it →TypeErroron every streaming request._patch_get_session_from_redissurvived byte-for-byte but its caller was replaced by upstream's → sessions written to Redis, read from the batch-delayed DB → multi-turn context loss.Changes
Resolver prompt (
carto-upstream-sync-resolver.yml):# CARTO PATCH). Replaces "preserve the BEHAVIOR, not necessarily the exact file versions".Fixer prompt (
carto-upstream-sync-ci-fixer.yml):Analyzer prompt (
carto-upstream-sync-customizations-analyzer.yml):PRESERVED_CARTOby WIRING, not string presence: an orphan / missing call site / broken data-flow isINCORRECTLY_DROPPEDeven when the pattern greps OK. It had reported the dead session read as PASS ✅.Docs (
CARTO_UPSTREAM_SYNC.md): new troubleshooting section with the three wiring-loss classes and detection commands.Ordering / dependencies
carto/mainwhen fix(ci): restore agent reaction to test failures via CARTO Feature Tests #125 merges. Both ultimately targetcarto/main.carto-features.ymlwith call-site/wiring patterns so the blocking manifest check catches orphans by grep too. Left out here because the open v1.92.0 sync PR rewrites that manifest and editing it now would conflict.Test plan
run:blocks passbash -n