Repository navigation
Conversation
|
@codex review |
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c87406c694
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Explanation of Change
This is Part 1 of a 3-part stacked PR series introducing a golden master regression harness for
SearchRouteroption search.The harness records, on
mainand before the upcoming in-memorySearchOptionsIndex, what the existingOptionsListUtilspipeline (createFilteredOptionListthroughgetSearchOptions) returns for a deterministic account, so later refactors can prove they change nothing.tests/utils/searchParity/generateSearchFixture.ts: a deterministic account with fixed IDs, unique timestamps and unique contact names. It covers 1:1 DMs, a group chat, a self-DM, own and others' workspace chats, an invoice room,#admins/#announce/custom rooms (one with an error, one whose workspace is gone), threads (with replies, hidden, empty, drafted, pinned), a task, expense reports, an archived DM, a DM with an unknown participant, and 40 filler contacts.Vít Novák,José Ñúñez,İsmail Yılmaz,Groß Straße,Łukasz Wójcik,Οδυσσεύς,Zoë 👩💻(ZWJ),AnaMaria(U+200B),Bob Builder(NBSP), NFD-decomposedRené Dupont,first.last+tag@x.com, an SMS login.JOSÉ,İSMAIL,Vít,René,Groß, an existing contact's phone and the own login, which must not invite).tests/unit/OptionsListUtilsSearchParityTest.ts: runs the production configs and records the results.searchRouterLocal(the SearchAutocompleteList local pass) andsearchRouterDisplayed(its ranking viacombineOrderingOfReportsAndPersonalDetails, theuserToInviteappend and the 20-row cap) for every query.Serialization
Each option is one diff-friendly line:
[flags]:[selected],[disabled],[bold],[gbr],[rbr],[archived].[alt: ...]:alternateText(chat previews in search results).[sub: ...]:subtitle, only for rooms and workspace chats, because that is what search matches for them.[accID: ...]: the option'saccountID, printed whenever it is non-zero (1:1 rows and contacts; groups and rooms carry none).[invitedLogin]: only onuserToInvite, when its login differs from its text.Fields the SearchRouter never reads (
participantsList,icons,policyID,ownerAccountID,item) are deliberately not serialized.Notes
getReportByIDis kept stable per reports collection with aWeakMap, asuseCallback(..., [allReports])does inuseFilteredOptions, so the preview cache is not invalidated between queries.hasMoreis recorded fromgetSearchOptions; no SearchRouter consumer reads it (the router'shasMorecomes fromuseFilteredOptions).npm test -- tests/unit/OptionsListUtilsSearchParityTest.ts -u) only against the pre-migration pipeline onmain, so the migration never becomes its own oracle.Fixed Issues
$ #103148
PROPOSAL:
Tests
Run
npm test -- tests/unit/OptionsListUtilsSearchParityTest.ts.Offline tests
N/A
QA Steps
N/A
PR Author Checklist
### Fixed Issuessection aboveTestssectionOffline stepssectionQA stepssectionAvatar, I verified the components usingAvatarare working as expected)npm run compress-svg)Avataris modified, I verified thatAvataris working as expected in all cases)Designlabel and/or tagged@Expensify/designso the design team can review the changes.mainbranch was merged into this PR after a review, I tested again and verified the outcome was still expected according to theTeststeps.