I'm not intricately familiar with the relevant code paths. Could you outline the incorrect logic vs your correct fix?

The idea - as far as I understood after analyzing the code - is that PetSpellMap is the server-side memory representation of the pet's spell book which is made persistent by saving to / loading from certain tables in the character database (pet_spell in particular). For performance reasons the database isn't updated on every change of the map but as a bulk operation on certain events , e. g. mounting/unmounting.

PetSpellMap basically stores the currently available spells along with anactive value (autocast on/off) and a state value (map entry needs to be written to / deleted from database). A map entry must not be erased from the map until it is deleted from the database. Instead it needs to be flagged for deletion by setting the state value to PETSPELL_REMOVED. Which means only the DB syncing code should be actually erasing entries fromPetSpellMap.

There are currently two occurrences of m_spells.erase() outside of the DB sync code. One doesn't seem to cause problems (unchanged), but the other (removed) can actually lead to the pathological case of erasing an entry from the PetSpellMap before it is also deleted from the DB. In fact it happens whenaddSpell cleans up PetSpellMap from lower ranked spells. The result is that PetSpellMap in memory and its representation on disk (DB) get out of sync. Multiple ranks of the same spell are still in pet_spell and get loaded from the DB on unmount / pet call, passed into addSpell and finally lead to autocast on/off not being properly restored.

I originally just noticed that the pet bar autocast issue only happens when there are entries for different ranks of the same spell in the pet_spell table (DB) and I was able to fix it (per pet) by manually deleting the lower ranks from the DB. Then I noticed how much effort the code puts into removing lower rank spells from PetSpellMap but that it fails to correctly update the DB. And finally I spotted the reason,

But why did my previous patch seem to have fixed autocast settings?
Well, it only fixed it for spells that are on the pet bar, not for those that are only toggled via spell book. The ugly fact that autocast settings are stored in two places (pet_spell for all spells, and character_pet for spells on the action bar) was able to obscure the root cause for a long time ...

👍 great analysis and good catch

Seems you integrated a slightly different fix. Guess that's alright. It reminds me to keep my work in a separate branch (3.3.5-my_work) to avoid merge conflicts when pulling "3.3.5" from upstream. :-)

You can simply base your branches on upstream/3.3.5 then push just the PR branch itself to your fork. No reason to keep your fork's main 3.3.5 branch up-to-date all the time.