[sc-12720] add "aiAnswersFilterObject" to settings and a method to se… #93
Conversation
WalkthroughAdds support for an AI-answers filtering object: new optional settings field and setter, a public client setter, README documentation for the filter object, and inclusion of the filter in the ai-answers POST payload; also minor type, helper, and config/version updates. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client as AddSearchClient
participant Settings as SettingsManager
participant API as ApiFetch
participant Server as AI-Answers Endpoint
Client->>Settings: setAiAnswersFilterObject(filter)
Settings-->>Client: (aiAnswersFilterObject stored)
Client->>API: requestAiAnswers(question, ...)
API->>Settings: read aiAnswersFilterObject
Note right of API #D3E4CD: Build POST payload\ninclude question + aiAnswersFilterObject
API->>Server: POST /ai-answers { question, aiAnswersFilterObject, ... }
Server-->>API: 200 OK / response
API-->>Client: response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
src/apifetch.ts (1)
344-345: Consider conditionally including the filter field for consistency.The current implementation always includes the
filterfield in the POST payload, even whenaiAnswersFilterObjectis undefined. This differs from the search path (lines 312-326), which conditionally includes the filter only when defined.Apply this diff to match the pattern used in the search implementation:
- apiInstance - .post(`https://${apiHostname}/v2/indices/${sitekey}/conversations`, { - question: settings?.keyword, - filter: settings?.aiAnswersFilterObject - }) + const payload: any = { + question: settings?.keyword + }; + + if (settings?.aiAnswersFilterObject) { + payload.filter = settings.aiAnswersFilterObject; + } + + apiInstance + .post(`https://${apiHostname}/v2/indices/${sitekey}/conversations`, payload)README.md (1)
535-551: Consider adding a complete usage example.The documentation clearly explains the filter structure, but it would be helpful to show the complete flow of setting the filter and then calling
aiAnswers().Consider adding a complete example like this:
// Set the filter before making the AI answers request var aiAnswersFilter = { and: [ { 'custom_fields.region': 'en-us' }, { not: { 'custom_fields.color': 'white' } } ] }; client.setAiAnswersFilterObject(aiAnswersFilter); // Then fetch AI answers with the filter applied client.aiAnswers('What products are available?', function(response) { console.log(response); });
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
README.md(1 hunks)src/apifetch.ts(1 hunks)src/index.ts(1 hunks)src/settings.ts(2 hunks)
🔇 Additional comments (3)
src/index.ts (1)
387-389: LGTM! Clean delegation to settings manager.The implementation correctly follows the established pattern used by
setFilterObjectand other similar methods.src/settings.ts (2)
57-57: LGTM! Appropriate type definition for the optional filter.The optional
aiAnswersFilterObjectproperty correctly mirrors the existingfilterObjectpattern and allows the feature to be used only when needed.
193-195: LGTM! Setter implementation is consistent.The setter follows the same straightforward pattern as
setFilterObject, maintaining consistency across the codebase.
…" instead of "replace()" with global regex
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/apifetch.ts (1)
342-369: Add empty object check for AI answers filter consistency.The search filter (line 325) uses
isEmptyObject()to exclude empty filter objects from the payload, but the AI answers filter (line 346) does not. Since both endpoints target the same API and the utility already exists, apply the same pattern to AI answers for consistency:.post(`https://${apiHostname}/v2/indices/${sitekey}/conversations`, { question: settings?.keyword, - filter: settings?.aiAnswersFilterObject + filter: isEmptyObject(settings?.aiAnswersFilterObject) ? undefined : settings?.aiAnswersFilterObject })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
package.json(1 hunks)src/apifetch.ts(6 hunks)tsconfig.json(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- tsconfig.json
- package.json
🔇 Additional comments (2)
src/apifetch.ts (2)
154-157: Good improvement for explicit boolean handling.The explicit
=== truecomparison makes the boolean coercion clear and defensive, ensuringfuzzyis only set totruewhenfuzzyRetryis strictlytrue.
53-57: No action needed—the type change is internal only.The
SourceDocumentsandConversationsApiResponseinterfaces are not exported from the module (both lack theexportkeyword), meaning they are internal implementation details. The type change fromDocument[]toSearchResponseDocument[]does not introduce breaking changes for external consumers since onlySearchResponseDocumentis exported and exposed to the public API. This internal refactoring aligns the type with the actual API response structure without affecting consumers.Likely an incorrect or invalid review comment.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src/apifetch.ts (1)
345-346: Consider filtering empty objects for consistency with other filter implementations.The search endpoint (lines 320-326) and customField filter (line 242) both use
isEmptyObject()to avoid sending empty filter objects. For consistency across the codebase, apply the same pattern toaiAnswersFilterObject:.post(`https://${apiHostname}/v2/indices/${sitekey}/conversations`, { question: settings?.keyword, - filter: settings?.aiAnswersFilterObject + filter: isEmptyObject(settings?.aiAnswersFilterObject) ? undefined : settings?.aiAnswersFilterObject })The
isEmptyObjectutility is already imported and used consistently elsewhere. However, if the AI Answers API explicitly handles empty objects or requires them for backward compatibility, this change may not be necessary.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/apifetch.ts(6 hunks)
🔇 Additional comments (3)
src/apifetch.ts (3)
156-156: LGTM - Simplified fuzzy retry logic.The simplified boolean expression correctly evaluates to
trueon retry (second call) andfalseon the initial call, which aligns with the fuzzy retry mechanism.
190-195: Good practice usingNumber.parseIntwith explicit radix.Using
Number.parseIntwith an explicit radix of 10 is more explicit and prevents potential octal interpretation issues.
222-224: LGTM - Single sort criterion handled correctly.The else branch correctly handles the case where
sortByis not an array or contains only one element.
haoAddsearch
deleted the
sc-12720/support-filter-parameter-in-ai-answers-endpoint
branch
|
2 participants
…t it.
Summary by CodeRabbit
New Features
Documentation
Chores