Repository navigation
fix: Volume sort with the 1h/24h windows 502s on production, three small token and launch fixes, a cached picture-ownership read - #79
Merged
Conversation
…d a failed list stays inline The window volumes live on the lateral w as v1 and v24; the ORDER BY named w1.v and w24.v, which do not exist. The column per window is now one table the test checks against the query. A failed list read shows an inline error with Try again, and picking a view on the home page changes the address with history.replaceState instead of re-rendering the page on the server, where a failing render unmounted it.
The chip rounded the label with fmtCompact(v, 0) while the value set was 2,500,000. It now uses fmtQuoteUnits like the other chips, which keeps two fraction digits.
…ape no longer leaves blank space under the footer Each row's sr-only label is absolutely positioned; with a static scroller it escaped the scroller's clipping and stretched the page's scrollable area.
The tint reads the stored logo with GetObject, which had no timeout. The read now carries a 3 second abort signal and a race that also covers a stalled body; a timeout is a miss, so the page falls back to the avatar tint.
The existing test checked the table against the query's aliases but not that listLaunchesPage uses it, so typing w1.v / w24.v back in would still pass. It now fails on those aliases.
… render #78's check for a copied hero picture reads bb_launch_meta with an ILIKE over the feed's picture keys. That is a scan: about 32 ms at production's size (5,400 rows, 24 keys, measured on a stand-in table), and the home page asks on every render. CodeRabbit flagged it on #78. The read is now cached for 30 s per set of pictures (memo, bounded, a failure is not cached). Stale is safe: a copy is judged against the earliest registration, which is older than any copy and already in the answer, and a launch with a new picture changes the set, so it is read fresh. Behaviour is otherwise unchanged, including failing closed when the read fails.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configuration
📒 Files selected for processing (13)
Included review availability: This review used your included allowance. 3 included reviews remain after this review. Your included PR review attempts over the past 7 days set your current allowance at 5 reviews per hour. 📝 WalkthroughWalkthroughThe changes update launchpad list navigation and retries, first-buy amount formatting, trade-history layout, S3 image reads, and launch query construction and caching. ChangesLaunchpad list loading
Launchpad display updates
S3 image reads
Launch query updates
Priority: ⬆️ High Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Bug fix Suggested reviewers: Merge Risk: ⚪ Minimal · up to The change fixes the volume-sort 502 for the 1h and 24h windows, corrects the first-buy chip formatting, adds list retry handling, and adds timeouts and caching to image and ownership reads. No merge-blocking risk was identified. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Comment |
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
1 participant
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.
What this fixes
1. The home list's Volume sort with the 1h or 24h window returns 502. This is live on production today.
GET /api/launch/list?sort=volume&window=1handwindow=24hboth return 502 on openlaunch.lol, and the home page then shows an empty list with "Try again". The window volumes live on the lateralwasv1andv24; theORDER BYnamedw1.vandw24.v, which do not exist. The column per window is now one table that a test checks against the query (a second test pins thatlistLaunchesPagereally uses it). Two things around it: a failed list read now shows an inline error with Try again, and picking a view on the home page changes the address withhistory.replaceStateinstead of re-rendering the page on the server, where a failing render unmounted the whole page.2. TWIG first-buy chip. The middle chip read "3M" while the value it sets is 2,500,000: the label was rounded with
fmtCompact(v, 0). It now usesfmtQuoteUnitslike the other chips, which keeps two fraction digits ("2.5M").3. Token page, trades list. The scroller is now a positioned ancestor. Each row's screen-reader label is absolutely positioned, and with a static scroller it escaped the scroller's clipping and stretched the page's scrollable area, leaving blank space under the footer after a long tape.
4. Token page, slow image store. The logo tint read the stored image with no timeout, so a slow store could hang the page. The read now has a 3 s abort signal and a race that also covers a stalled body; a timeout counts as a miss and the page falls back to the avatar tint.
5. Feed, picture-ownership read (CodeRabbit's note on #78). The check for a copied hero picture reads
bb_launch_metawith anILIKE, which is a scan: about 32 ms at production's size (5,400 rows, 24 keys, measured on a stand-in table), and the home page asks on every render. It is now cached for 30 s per set of pictures (the cache is bounded and a failure is not cached). Stale is safe: a copy is judged against the earliest registration, which is older than any copy and already in the answer, and a launch with a new picture changes the set, so it is read fresh. Behaviour is otherwise the same, including failing closed. (The "about 350 rows" in #78's deploy notes was my local copy of the data; production has far more, which is why this matters.)Verified
tscandeslint: 0 problems. 1,055 tests: 1,045 pass, 10 skipped, 0 fail.LaunchList.tsx(both sides added a line next to each other); I kept both.Deploy notes
No schema change and no new environment variable. It can ship with #78 or after it. This branch replaces
fix/home-live-bugs, which was based on the old main and is left as it was.Summary by CodeRabbit