Conversation
|
Quick links (staging server):
Login:
Archive:SVG tester:Number of differences (graphers): 0 ✅ Edited: 2026-02-13 20:42:23 UTC |
55d0d34 to
65df6c4
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 65df6c4549
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
865a90e to
7d5e1a7
Compare
| getItemUrl({ item }) { | ||
| const itemUrl = prependSubdirectoryToAlgoliaItemUrl(item) | ||
| return itemUrl | ||
| const algoliaItemTemplate: AutocompleteSource<BaseItem>["templates"] = { |
There was a problem hiding this comment.
Lifted this out of AlgoliaSource so that it can be reused by createProfileSource
7d5e1a7 to
762fe23
Compare
mlbrgl
left a comment
There was a problem hiding this comment.
submitting partial review, still pondering over the use of extractFiltersFromQuery in Autocomplete
Besides those inline comments, we're going need some updates (either here or in #6103) to get profiles to use the new indexing pipeline (vs the markdown)
baker/algolia/utils/pages.ts
Outdated
|
|
||
| const getThumbnailUrl = ( | ||
| gdoc: OwidGdocPostInterface | OwidGdocDataInsightInterface, | ||
| gdoc: OwidGdocBaseInterface, |
There was a problem hiding this comment.
we could keep a tighter type here to benefit maintain compile type checking
There was a problem hiding this comment.
we should delete profile records in deleteGdoc too
There was a problem hiding this comment.
we might want to either add support for profiles in getPreviewGdocIndexRecords or update the message that says "not indexed" (maybe something the lines of "profiles are not supported in preview mode currently")
site/search/SearchWritingResults.tsx
Outdated
| queryFn: (liteSearchClient, state, offset, length) => { | ||
| return queryProfiles(liteSearchClient, state, offset, length) | ||
| }, | ||
| firstPageSize: 4, |
There was a problem hiding this comment.
this throws off the display of the topic/profile section, which should render - for instance, in the absence of articles - no more than 6 boxes (e.g. "china" + co2 now renders 8).
There was a problem hiding this comment.
True! Looking into this, I'm not sure it's working as intended?
I thought I could fix this issue by writing (pseudocode):
const profiles = profilesQuery(size: 2)
slotsLeft = 6 - Math.min(profiles.totalResults, 2)
const articles = articlesQuery(size: 2)
slotsLeft -= Math.min(articles.TotalResults, 2)
const topicPages = topicQuery(size: slotsLeft)
but topicPages is always fetching 6, so I think in the current (production) code, noArticles is probably not working correctly, either?
There was a problem hiding this comment.
If no articles are returned (e.g. "co2" + tag:china)
- requests 2 articles (gets none) and consequently fetches 6 topic pages.
If articles are returned (e.g. "co2"), then only 2 topic pages are fetched initially, then 4.
So that works as expected as far as I can tell.
And the interleaving logic makes sure than the amount of topics vs articles is always balanced in the multi column layout (e.g. if there were 2 articles and 10 topics total for a query, then most topics would end up in the overflow).
|
nice, thanks for the integration! Nothing major but there are a couple of things we might to look at before merging. |
762fe23 to
2817e33
Compare
2817e33 to
a14e155
Compare
a14e155 to
8604cc5
Compare
8604cc5 to
3aaae1a
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0d3b9d202f
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return queryArticles(liteSearchClient, state, offset, length) | ||
| }, | ||
| firstPageSize: 2, | ||
| firstPageSize: 4 - profileSlots, |
There was a problem hiding this comment.
Freeze article page sizing before running paginated query
When showProfiles is true, profileSlots is 0 on the first render, so the articles query starts with firstPageSize=4; once profiles load, firstPageSize drops (to 2/3), but the React Query key is unchanged, so the existing infinite-query pages are reused with a different offset formula. That mismatch can produce duplicated/skipped article hits on fetchNextPage and overfill the first results block whenever profiles exist.
Useful? React with 👍 / 👎.
0d3b9d2 to
1138333
Compare

Context
Part of #5918
Figma
Screenshots / Videos / Diagrams
Testing guidance
Step-by-step instructions on how to test this change
indexPagesToAlgoliaChecklist
Before merging