fix(api): properly dedup name list responses#4337
fix(api): properly dedup name list responses#4337MasterPtato wants to merge 1 commit intofix-ups-formalize-subjectsfrom
Conversation
|
This change is part of the following stack: Change managed by git-spice. |
|
🚅 Deployed to the rivet-pr-4337 environment in rivet-frontend
|
|
PR Review: fix(api): properly dedup name list responses The core idea is right -- using IndexMap/IndexSet as the fanout accumulator to deduplicate names naturally is cleaner than the previous Vec-based approach. However, there are two bugs introduced. Bug 1: Limit applied before sort in list_names.rs and runners.rs In both files, .take(limit) runs on the unsorted fanout result before sort_keys()/sort(). IndexMap/IndexSet preserves insertion order (non-deterministic across fanout responses), so the limit is applied to an arbitrary subset. The page is sorted within itself, but may not be the right first-N alphabetical entries. The fix: collect all (dedup via IndexMap/IndexSet) -> sort -> truncate. Both types support truncate() in-place, so the sort-then-truncate idiom used in the old Vec code should be preserved: For list_names.rs: sort_keys() then truncate(query.limit.unwrap_or(100)) Bug 2: Cursor filtering removed in list.rs (actor_ids path) The PR removes cursor-based pagination filtering for the actor_ids path with no apparent replacement. Unless fetch_actors_by_ids now handles cursor filtering internally when given Some(limit), the cursor query param is silently ignored for this path and pagination always returns the same first page. This also appears unrelated to the dedup fix -- was this intentional? Minor: Redundant truncation in list.rs After passing Some(limit) to fetch_actors_by_ids, actors.truncate(limit) is a no-op if the fetch already respects the limit. Summary
|
MasterPtato
force-pushed
the
fix-ups-formalize-subjects
branch
from
c8e5674 to
73da062
Compare
MasterPtato
force-pushed
the
fix-api-properly-dedup-name-list
branch
from
c571c59 to
5950673
Compare
1 participant
Description
Please include a summary of the changes and the related issue. Please also include relevant motivation and context.
Type of change
How Has This Been Tested?
Please describe the tests that you ran to verify your changes.
Checklist: