Skip to content

SCIX-891 feat(search): tag Solr requests with a ui_tag - #932

Open
thostetler wants to merge 1 commit into
adsabs:masterfrom
thostetler:feature/scix-891-ui-tag-solr-queries
Open

thostetler wants to merge 1 commit into
adsabs:masterfrom
thostetler:feature/scix-891-ui-tag-solr-queries

Conversation

@thostetler

@thostetler thostetler commented Sep 10, 2026

Copy link
Copy Markdown
Member

Solr traffic was anonymous in backend logs: a search, a facet, and a citations lookup all looked alike.

  • Tag derives from the React Query key namespace
  • namespace is required on useSearch and useSearchInfinite
  • Ten reused call sites name their own surface
  • Literal tags on the paths with no query key
  • Fixes the search page reading a foreign numFound

@github-actions

github-actions Bot commented Sep 10, 2026

Copy link
Copy Markdown

Bundle size

Shared by all pages: 625.1 kB (+0.5 kB) ⚪

No route changed by more than 1 kB. ✅

First load = polyfills + shared _app chunks + route chunks, gzipped.

@codecov

codecov Bot commented Sep 10, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 87.93970% with 24 lines in your changes missing coverage. Please review.
✅ Project coverage is 68.6%. Comparing base (936dcfb) to head (2b8fe53).
⚠️ Report is 5 commits behind head on master.

Files with missing lines Patch % Lines
src/api/search/search.ts 82.4% 18 Missing ⚠️
src/lib/orcid/useOrcidProfile.ts 25.0% 3 Missing ⚠️
src/api/vis/vis.ts 93.8% 1 Missing ⚠️
src/lib/orcid/useAddWorks.ts 50.0% 1 Missing ⚠️
src/lib/orcid/useUpdateWork.ts 50.0% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff            @@
##           master    #932     +/-   ##
========================================
+ Coverage    67.9%   68.6%   +0.8%     
========================================
  Files         378     380      +2     
  Lines       44165   44304    +139     
  Branches     2380    2407     +27     
========================================
+ Hits        29954   30366    +412     
+ Misses      14163   13889    -274     
- Partials       48      49      +1     
Files with missing lines Coverage Δ
src/api/search/types.ts 100.0% <100.0%> (ø)
src/api/search/ui-tags.ts 100.0% <100.0%> (ø)
src/api/vault/vault.ts 56.3% <100.0%> (+7.5%) ⬆️
...orms/AssociatedArticles/AssociatedArticlesForm.tsx 67.6% <100.0%> (ø)
...ckForms/MissingReferences/MissingReferenceForm.tsx 60.5% <100.0%> (ø)
src/lib/serverside/absCanonicalization.ts 86.7% <100.0%> (+1.7%) ⬆️
src/lib/useBatchedSearch.ts 96.5% <100.0%> (+0.1%) ⬆️
src/api/vis/vis.ts 69.8% <93.8%> (+2.0%) ⬆️
src/lib/orcid/useAddWorks.ts 15.5% <50.0%> (-0.1%) ⬇️
src/lib/orcid/useUpdateWork.ts 14.2% <50.0%> (-0.1%) ⬇️
... and 2 more

... and 5 files with indirect coverage changes

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@thostetler
thostetler marked this pull request as ready for review September 11, 2026 12:33
Copilot AI lite review requested due to automatic review settings September 11, 2026 12:33

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Critical cache-key defects can suppress correctly tagged requests, and the shared-cache test gap remains unresolved.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR adds ui_tag route metadata to client and server-side Solr requests for better request attribution.

Changes:

  • Adds client-side route detection and SSR route tagging.
  • Threads tags through search and visualization hooks.
  • Adds route-tagging types and tests.

Review findings:

  • Critical (3 votes): Search query keys omit uiTag, allowing incorrect cache reuse.
  • Critical (1 vote): Results-graph query keys have the same cache identity issue.
  • Moderate (2 votes): The route-isolation test uses separate query clients.
  • Nit (1 vote): The ui_tag type comment overstates typo protection.
File summaries
File Reviewed changes
src/pages/user/settings/export.tsx Tags export-page SSR searches.
src/pages/search/exportcitation/[format].tsx Tags citation-export SSR searches.
src/pages/search/citation_helper.tsx Tags citation-helper SSR searches.
src/pages/feedback/missingrecord.tsx Tags missing-record SSR lookups.
src/pages/abs/[id]/exportcitation/[format].tsx Supplies the dynamic route pattern.
src/lib/useClientSideRoute.ts Resolves client route patterns.
src/lib/serverside/absCanonicalization.ts Tags abstract canonicalization requests.
src/lib/serverside/__tests__/absCanonicalization.test.ts Tests SSR route tagging.
src/api/vis/vis.ts Tags results-graph requests; cache keys omit the tag.
src/api/vis/vis.test.tsx Tests visualization request tagging.
src/api/search/types.ts Adds ui_tag typing and its explanatory comment.
src/api/search/search.ts Propagates tags; search cache keys omit the tag.
src/api/search/__tests__/uiTag.test.tsx Tests hook tagging but uses separate query clients.
src/api/search/__tests__/search.test.ts Tests request parameter tagging.
Review details

Suppressed comments (1)

src/api/search/types.ts:30

  • The new comment overstates the type safety here: the [key: string] index signature on the next line still allows misspellings such as ui_tga, so declaring ui_tag explicitly does not stop a typo from being sent. Please either describe this as discoverability/value typing or change the type design if compile-time rejection is required. Confidence: high.
  // Explicit field so a typo doesn't silently vanish into the index signature.
  ui_tag?: string;
  • Files reviewed: 14/14 changed files
  • Comments generated: 3
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread src/api/search/search.ts Outdated
Comment thread src/api/vis/vis.ts Outdated
Comment thread src/api/search/__tests__/uiTag.test.tsx Outdated
@thostetler
thostetler marked this pull request as draft September 11, 2026 20:54
@thostetler
thostetler force-pushed the feature/scix-891-ui-tag-solr-queries branch 2 times, most recently from 538b14e to 716d6f4 Compare September 14, 2026 20:40
@thostetler thostetler changed the title SCIX-891 feat(search): tag Solr requests with originating route SCIX-891 feat(search): tag Solr requests with a ui_tag per surface Sep 14, 2026
@thostetler
thostetler force-pushed the feature/scix-891-ui-tag-solr-queries branch from 716d6f4 to 6970bfc Compare September 14, 2026 20:44
Solr traffic was anonymous in backend logs. Every request now carries a
ui_tag derived from its React Query key namespace.
@thostetler
thostetler force-pushed the feature/scix-891-ui-tag-solr-queries branch from 6970bfc to 2b8fe53 Compare September 14, 2026 20:46
@thostetler thostetler changed the title SCIX-891 feat(search): tag Solr requests with a ui_tag per surface SCIX-891 feat(search): tag Solr requests with a ui_tag Sep 14, 2026
@thostetler
thostetler marked this pull request as ready for review September 14, 2026 21:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants