Repository navigation
three corrections from auditing my own nova work after it deployed - #3182
Open
Flotapponnier wants to merge 3 commits into
Open
Flotapponnier wants to merge 3 commits into
Flotapponnier wants to merge 3 commits into
Conversation
Found by reading the gauge after deploying: perp_leverage_max published 500.001 for Nova where Gains reads 500. Raw integers at 1e3: gains 300/313 min 500000 max 500000 min == max, genuinely fixed nova 300/313 min 500000 max 500001 a band one thousandth wide My own comparison script printed both as "500.00" because it formatted to two decimals. The difference was in the data from the first read and the display hid it, so three things shipped wrong in #3168: - the model carried maxLeverage 500 for a row whose gauge says 500.001, which had this page and bench 295 disagreeing about one venue - the row called itself "fixed, same terms as Gains". It is neither: its bounds do not coincide, so it is not fixed, and its ceiling is not Gains'. Now "x500 degen, Gains' terms at 500.001x" - the bench formula claimed "leverage bounds identical to Gains". The standard pair's are; the degen pair's maximum is not And the test asserted the two maxima were EQUAL. It passed because both rows carried the same wrong number, which is the failure mode worth naming: a test comparing two values I wrote by hand confirms I was consistent, not that I was right. It now pins 500 and 500.001 separately, asserts Gains' bounds coincide while Nova's do not, and the harness checks the same two figures against the contract on OCB_LIVE. The test log prints leverage to three decimals now, since the contract stores it at 1e3 and rounding to two is the thing that cost this. Nobody trades the difference. It is carried because the published gauge carries it, and because a fork that edited one digit of one parameter edited it on purpose.
Florent noticed that /products/nova-markets shows one bench, and that the one it shows says "awaiting". The first half is mine. #3157 closed the duplicate page with a hardcoded permanentRedirect on slug === "nova". That fixed the URL and nothing else: a product page's bench appearances are keyed by PROVIDER SLUG, and Nova has two. Bench 030 lists it as "nova-markets", benches 041, 116 and 295 as "nova". So the redirect sent readers to a page that lists bench 030 and none of the three the venue was just added to. The repo already had the mechanism, PRODUCT_ALIASES in providers.ts, and its own comment describes this exact situation for the -cold slugs: "Folding them here collapses their bench appearances onto the vendor's page and leaves one page per vendor". It does strictly more than the redirect, because canonicalize() moves the bench rows too, and the generic alias-to-canonical 308 already in the route then handles the URL without a special case. Both hardcoded redirects come out. The test I wrote with that redirect asserted the hardcoded line was present, so it pinned the mechanism I had reached for rather than the property that matters, and it would have passed forever while the bench rows stayed stranded. It now asserts canonicalize("nova") resolves to nova-markets, that the canonical slug is a fixed point so the redirect cannot loop, and that the generic 308 is still in both call sites. The two-call-site check matches on the shared comparison rather than a variable name, because the two sites spell the variable differently. Not fixed here, because it is a judgement on a bench I did not build: bench 030 renders Nova as "$0, ranked 105 of 105" while the product page says "awaiting". The product page's hasData requires p50 !== 0 unless the spec sets zero_is_a_value, and bench 030 does not set it, so 34 of its 105 providers read as missing data on their product pages while the bench ranks them on a measured zero.
Florent asked why /products/nova-markets lists one bench. The alias was only half of it. canonicalize() keys provider profiles by the canonical slug, so folding nova onto nova-markets is what moves the bench appearances. That landed in #3176 as a sitemap hotfix, and the page still listed bench 030 alone. The rest is providers-vN in providers.ts: a fourth cache key, holding the per-product profile index. Its own history says what happens when it is missed, in the same words every time: "GetBlock gains nine appearances ... so the v14 list would keep /products/getblock without them." Nova gains appearances on 041, 116 and 295, and the alias changes which profile they key onto, so both halves need it. v17. I bumped three keys on this work and there are four. The cache-key test I wrote in #3150 knew two, I extended it to three in #3168, and it still did not know about this one because it only reads spec.ts. It now reads providers.ts too, asserts the four it knows are each defined once, and fails on any OTHER unstable_cache key in either file, so a fifth is reported rather than discovered the way the fourth was. Also drops the duplicate alias: #3176 added the same mapping while this branch was open, and the reasoning about stranded bench rows is folded into the surviving entry. Worth recording for whoever reads this next: the live page was served from the worker's providers.json, not from this cache, so re-publishing the blob is the first thing that matters and the key bump covers the fallback. The worker clones dev at image build and was running 15:47 code, two commits before the alias.
This branch has not been deployed
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.
Replaces #3171 and #3178, both branched before #3168 was squash-merged. Three things, each found by looking at what shipped rather than at the code.
1. Nova's degen pair is 500.001x, not 500x
Read off the deployed gauge:
perp_leverage_maxpublished 500.001 for Nova where Gains reads 500. Raw at 1e3, Gains 300/313 is min 500000 max 500000 (genuinely fixed), Nova is min 500000 max 500001.My own comparison script printed both as
500.00because it formatted to two decimals. So the model carried 500 for a row whose gauge says otherwise, the row called itself "fixed, same terms as Gains" when it is neither, and bench 295's formula claimed identical leverage bounds.The test was the worst part. It asserted the two maxima were equal, and passed, because both rows carried the number I had typed by hand. A test comparing two values I wrote confirms I was consistent, not that I was right. It now pins 500 and 500.001 separately and the harness checks both against the contract under
OCB_LIVE=1. The live test's log prints three decimals, because the contract stores leverage at 1e3 and rounding to two is exactly what this cost.2. My redirect moved the URL and stranded the bench rows
This is what Florent spotted: /products/nova-markets lists one bench.
#3157 closed the duplicate page with a hardcoded
permanentRedirect. A product page's bench appearances are keyed by provider slug, and Nova has two:nova-marketsfor bench 030,novafor 041, 116 and 295. The redirect sent readers to a page listing the first and none of the rest.PRODUCT_ALIASESis the mechanism, becausecanonicalize()keys profiles by the canonical slug and so moves the appearances too. It landed in #3176 as a sitemap hotfix while this branch was open; the duplicate entry is removed here and the reasoning folded into the survivor. The hardcoded redirect comes out, since the generic alias-to-canonical 308 already in that route does it.That same redirect is what failed the prod deploy at 16:06: the sitemap still listed
/products/nova, the smoke test rejected a 308, and the deploy auto-rolled back (163/164 OK). The guard worked exactly as built. It was my change that tripped it, on someone else's release.The test I wrote with that redirect asserted the hardcoded line was present, so it pinned the mechanism I reached for rather than the property that matters, and would have passed forever while the rows stayed stranded. It now asserts
canonicalize("nova")resolves tonova-markets, that the canonical slug is a fixed point so the redirect cannot loop, and that the generic 308 is in both call sites.3. There is a fourth cache key
providers-vNin providers.ts holds the per-product profile index. Its own history says what happens when it is missed, in the same words every time:I bumped three keys across this work. There are four. The cache-key test I wrote in #3150 knew two, I extended it to three in #3168, and it still missed this one because it only reads
spec.ts. It now readsproviders.tsas well, and fails on any unstable_cache key in either file that it does not track, so a fifth is reported rather than discovered the way the fourth was. Verified against a mutation that adds one.On the live symptom
The page was served from the worker's
providers.json, not from that cache. The worker clonesdevat image build and was running 15:47 code, two commits before the alias. I rebuilt it; it is now on current dev and its sweep republishes the blob. The key bump covers the fallback path.Still not fixed, still your call
Bench 030 renders Nova as "$0, ranked 105 of 105" while the product page says "awaiting / data warming up".
hasDatarequiresp50 !== 0unless the spec setszero_is_a_value, and bench 030 does not: 34 of its 105 providers read as missing data on their product pages while the bench ranks them on a measured zero. Nova's p50 is 0 because it had 0 fills on the last complete UTC day; its 7-day mean is $1.42.The one-line fix is
zero_is_a_value: trueon that spec, but the flag also feedscomputeFieldStats, andstats.tsrecords what happened last time it was switched on somewhere (bench 282 printed "Best $0"). The guard added then issuccessRate > 0, which cannot discriminate here because bench 030'ssuccessquery is a harness-freshness check returning 1 for every builder.Gate: validate 253 specs, typecheck, lint at its 9 pre-existing warnings, test 715 pass, go build/vet/test green.