Skip to content

Search: Fix modal rejecting valid responses with content type "docs"#3456

Merged
reakaleek merged 2 commits into
mainfrom
icy-lingonberry
Jun 2, 2026
Merged

Search: Fix modal rejecting valid responses with content type "docs"#3456
reakaleek merged 2 commits into
mainfrom
icy-lingonberry

Conversation

@reakaleek

@reakaleek reakaleek commented Jun 2, 2026

Copy link
Copy Markdown
Member

Why

The upstream search contract (Elastic.Internal.Search.Contract) changed the field name from type to content_type and updated the value from "doc" to "docs" on indexed documents. The frontend Zod schemas still declared type: z.enum(['doc', 'api']), so SearchResponse.parse() threw a ZodError on every result item — rejecting the entire valid 200 response and showing "Error loading search results" in the codex search modal.

What

  • Renamed typecontent_type and updated z.enum(['doc', 'api']) to z.enum(['docs']) in both useModalSearchQuery and useNavigationSearchQuery.
  • Removed the now-dead result.type === 'api' branch from both result list components; codex is docs-only so typePrefix is always 'Docs'.
  • Added an else if (query.error) branch to the error-logging effect in both hooks, so ZodError/parse failures are no longer silent — they now emit logError to OTLP and console.error to devtools.
  • Updated the test fixture to use content_type: 'docs'.

The navigation-search contract changed the result item type value from
"doc" to "docs", but the frontend Zod schemas still enumerated ['doc',
'api'], causing SearchResponse.parse() to throw on every result item and
surface "Error loading search results" even for healthy 200 responses.

Also adds a console.error + logError branch for non-ApiError query
failures so ZodError/parse mismatches are no longer completely silent.

Co-Authored-By: Claude Sonnet 4.6 (1M context) <noreply@anthropic.com>
@reakaleek reakaleek requested a review from a team as a code owner June 2, 2026 16:00
@reakaleek reakaleek requested a review from Mpdreamz June 2, 2026 16:00
@reakaleek reakaleek added the bug label Jun 2, 2026
@reakaleek reakaleek temporarily deployed to integration-tests June 2, 2026 16:00 — with GitHub Actions Inactive
@coderabbitai

coderabbitai Bot commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: cf479a43-5f19-4282-8eb2-7c820bf676a9

📥 Commits

Reviewing files that changed from the base of the PR and between 6016f42 and 2ad0aeb.

📒 Files selected for processing (3)
  • src/Elastic.Documentation.Site/Assets/web-components/ModalSearch/useModalSearchQuery.ts
  • src/Elastic.Documentation.Site/Assets/web-components/NavigationSearch/NavigationSearchTelemetry.test.tsx
  • src/Elastic.Documentation.Site/Assets/web-components/NavigationSearch/useNavigationSearchQuery.ts
🚧 Files skipped from review as they are similar to previous changes (2)
  • src/Elastic.Documentation.Site/Assets/web-components/ModalSearch/useModalSearchQuery.ts
  • src/Elastic.Documentation.Site/Assets/web-components/NavigationSearch/useNavigationSearchQuery.ts

📝 Walkthrough

Walkthrough

This PR aligns search result type contracts and breadcrumb rendering across Modal and Navigation search components. The SearchResultItem type enum is narrowed from ['doc', 'api'] to ['docs'] in both hooks. Breadcrumb rendering logic in both components is updated to use a fixed 'Docs' prefix instead of deriving it from result.type, removing that field from memoization dependencies. Error handling is enhanced in both hooks to log non-API parsing errors separately via logError with error metadata. Test fixtures are updated to reflect the new schema.

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Title check ✅ Passed The title accurately reflects the main change: fixing the search modal to accept responses with the updated 'docs' content type.
Description check ✅ Passed The description clearly explains the root cause (schema mismatch), what was changed (field rename and enum update), and why (to match upstream contract changes).
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
✨ Simplify code
  • Create PR with simplified code
  • Commit simplified code in branch icy-lingonberry

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot 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.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
src/Elastic.Documentation.Site/Assets/web-components/ModalSearch/useModalSearchQuery.ts (1)

38-45: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Keep the result type parser rollout-compatible.

Accepting only 'docs' means any still-legacy 'doc' payload from a staggered deploy, stale cache, or partial reindex will keep failing with the same parse error. Parse both values here and normalize later so search stays up during the transition.

Suggested change
 const SearchResultItem = z.object({
-    type: z.enum(['docs']),
+    type: z.enum(['doc', 'docs']),
     url: z.string(),
     title: z.string(),
     description: z.string(),
     score: z.number(),
     parents: z.array(SearchResultItemParent),
 })
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@src/Elastic.Documentation.Site/Assets/web-components/ModalSearch/useModalSearchQuery.ts`
around lines 38 - 45, The SearchResultItem Zod schema currently only accepts
type 'docs', which will reject legacy 'doc' payloads during a rollout; update
the parser for SearchResultItem.type to accept both 'docs' and 'doc' (e.g.,
using an enum/union for ['docs','doc']) and then normalize the value downstream
where SearchResultItem is consumed so callers always see a single canonical
value (e.g., map 'doc' -> 'docs') to maintain compatibility during staggered
deploys.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Outside diff comments:
In
`@src/Elastic.Documentation.Site/Assets/web-components/ModalSearch/useModalSearchQuery.ts`:
- Around line 38-45: The SearchResultItem Zod schema currently only accepts type
'docs', which will reject legacy 'doc' payloads during a rollout; update the
parser for SearchResultItem.type to accept both 'docs' and 'doc' (e.g., using an
enum/union for ['docs','doc']) and then normalize the value downstream where
SearchResultItem is consumed so callers always see a single canonical value
(e.g., map 'doc' -> 'docs') to maintain compatibility during staggered deploys.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Enterprise

Run ID: d508d3f5-2341-4bf8-990d-95a6709641e9

📥 Commits

Reviewing files that changed from the base of the PR and between e010a12 and 6016f42.

📒 Files selected for processing (5)
  • src/Elastic.Documentation.Site/Assets/web-components/ModalSearch/ModalSearchResultsList.tsx
  • src/Elastic.Documentation.Site/Assets/web-components/ModalSearch/useModalSearchQuery.ts
  • src/Elastic.Documentation.Site/Assets/web-components/NavigationSearch/NavigationSearchTelemetry.test.tsx
  • src/Elastic.Documentation.Site/Assets/web-components/NavigationSearch/SearchResultsList.tsx
  • src/Elastic.Documentation.Site/Assets/web-components/NavigationSearch/useNavigationSearchQuery.ts

@reakaleek

Copy link
Copy Markdown
Member Author

@copilot this fix is wrong. The schema changed. It's now content_type with the value "docs".

Just change the key and fix the value in the zod schema

Copilot AI commented Jun 2, 2026

Copy link
Copy Markdown
Contributor

@copilot this fix is wrong. The schema changed. It's now content_type with the value "docs".

Just change the key and fix the value in the zod schema

Fixed in the latest commit — renamed the key from type to content_type (keeping the value 'docs') in the Zod schema in both useModalSearchQuery.ts and useNavigationSearchQuery.ts, and updated the test fixture to match.

Copilot AI temporarily deployed to integration-tests June 2, 2026 16:58 Inactive
@reakaleek

Copy link
Copy Markdown
Member Author

Merging because this is broken on live elastic codex.

@reakaleek reakaleek merged commit ced14d0 into main Jun 2, 2026
25 checks passed
@reakaleek reakaleek deleted the icy-lingonberry branch June 2, 2026 18:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants