Repository navigation
Filter the approvals queue in place instead of navigating to it - #38
Merged
Merged
Conversation
The tabs on /approvals were links to /approvals?filter=…, so every click was a fresh server round-trip: the page blanked, the queue was fetched again and the scroll position went back to the top — to show rows the browser already had. Four tabs meant four URLs for one screen. The page now asks for the whole pending queue once and the tabs pick from it in the browser. Switching costs a re-render and no request at all. Each row carries the bucket it belongs to, computed in the same SQL that produces the counts on the tabs, so a client-side tab can never disagree with the badge beside it. Recomputing the classification in the browser from `action` and `draft_body` would have been a second implementation of that rule, free to drift. The URL still tracks the tab, via replaceState rather than a navigation, so /approvals?filter=research still deep-links and a reload lands back on the tab you were reading. replaceState and not pushState: a filter is a view of one page, so Back should leave the queue rather than walk you through the tabs you clicked on the way in. The fetch is one page of the queue rather than one page per tab, so the limit is the API's own ceiling of 200 — production's queue is ~75 rows. Past that the counts still tell the truth and the page says it is showing a subset. Verified in a real browser against a production build: clicking a tab swaps the cards, updates the URL, adds no history entry and issues no document request, and a marker set on `window` survives the click. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
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.
The problem
The filter tabs on
/approvalswere<Link>s to/approvals?filter=…, so clicking one was a navigation, not a filter. The page blanked, the queue was fetched again, and the scroll position went back to the top — to show rows the browser already had. Four tabs meant four URLs for one screen.What changed
The page asks for the whole pending queue once and the tabs pick from it in the browser. Switching tabs costs a re-render and no request at all.
bucketon every row. Each recommendation now carriesready/needs_draft/research, computed in the same SQL that produces the counts on the tabs, so a client-side tab can never disagree with the badge beside it. Recomputing that in the browser fromactionanddraft_bodywould be a second implementation of a rule that already exists, free to drift from it.replaceStaterather than a navigation, so/approvals?filter=researchstill deep-links and a reload lands back on the tab you were reading.replaceStateand notpushState: a filter is a view of one page, so Back should leave the queue rather than walk you through the tabs you clicked on the way in.The bucket expression sits in the
SELECT, ahead of theWHEREclause the filter narrows, so its placeholders bind first. That reordering is the classic way this breaks, and there is a test for it.Verification
bun run check— format, typecheck, 847 tests pass.next buildforapps/web(the root typecheck does not cover it).?filter=researchdeep-links to the Research tab showing its 3 cardswindowsurvives the click — the page was never replaced?filter=readyhistory.lengthdoes not grow, and no new document request is issued🤖 Generated with Claude Code