fix(backfill): merge-preserving regen, real recipe yields, single-flight resolve#22
Merged
Merged
Conversation
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.
Fixes three defects found in a code review of the price-backfill toolchain.
Silent override loss on in-place regeneration
backfill.py --out-sql(which defaults to--existing-sql, documented as in-place regeneration) emitted only rows derived from the current candidates run. Any currently-overridden item absent fromcandidates.csv(BoP gear, legendaries, or simply a narrower candidate query) silently disappeared from the regenerated SQL, reverting to default AH pricing.write_outputs()now seeds the output from the existing overrides and overlays only items present in the new records, mirroringderive.py's merge. Regression test added.Recipe yield hardcoded to 1
spelldbc.pyrecorded every crafting recipe with yield 1, so multi-output recipes (e.g. basePoints=4 -> 5 per cast) were over-priced by the yield factor inrecipe_prices.py, and the error propagated recursively into higher-tier recipes using the output as a reagent. Yield is now extracted asEffectBasePoints + 1(the 3.3.5a minimum-yield formula; variable rolls use the minimum, so derived prices are never inflated). Non-positive yields (bogus data) skip the recipe with a stderr warning instead of silently pricing at yield 1. Tests cover yield-1, fixed multi-yield, recursive propagation, and the negative-basePoints skip.Lock held across network I/O
BuildIdState.note_resultheld the state lock across the sentinel fetch and buildId re-resolve, collapsing worker concurrency to 1 during every redeploy check. The lock now guards only shared state; a_resolvingflag (held until the buildId update completes) keeps the re-resolve single-flight. Concurrency tests added (verified to fail against the pre-fix code).Test suite: 51 passed.