-
Notifications
You must be signed in to change notification settings - Fork 0
feat: Cognitive Search infrastructure with env-based switching, mock LiQE/Lunr engine, Storybook fixes, and CI hardening #237
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The pull request #237 has too many files changed.
The GitHub API will only let us fetch up to 300 changed files, and this pull request has 1160.
- Created @cellix/mock-cognitive-search package with in-memory implementation - Created @sthrift/service-cognitive-search package with auto-detection - Added domain interfaces and search index definitions - Implemented GraphQL schema and resolvers for item listing search - Added service registration and environment configuration - Core search functionality working end-to-end - Event handlers temporarily disabled due to build issues TODO: Fix module resolution issues for event handlers TODO: Complete integration tests and documentation
…zure support - Add @cellix/mock-cognitive-search package with in-memory Azure-like API - Add @sthrift/service-cognitive-search with environment-driven implementation selection - Implement domain events for item listing updates/deletions - Add search index helpers with retry logic and hash-based change detection - Integrate ServiceCognitiveSearch into API service registry - Configure environment variables for mock/Azure switching Foundation for cognitive search with mock implementation complete. Real Azure integration and comprehensive testing pending.
…lback - Add AzureCognitiveSearch client with full Azure SDK integration - Implement automatic switching between Azure and mock implementations - Add comprehensive documentation and Azure vs mock comparison example - Fix event handler integration with proper transaction handling - Add Azure environment variable configuration support - Ensure graceful fallback to mock when Azure credentials unavailable
- Add comprehensive data seeding service to mock-mongodb-memory-server - Seed 5 mock users and 6 mock item listings with proper ObjectId relationships - Create database-driven cognitive search example replacing hardcoded samples - Integrate ItemListingSearchApplicationService into application services - Add cognitive search to existing GraphQL resolvers with fallback support - Ensure all mock data follows ShareThrift domain model and is interconnected - Add MongoDB indexes for performance and proper data relationships - Update documentation to reflect database-driven approach All mock data is now seeded into MongoDB and queried from database, satisfying requirements for interconnected, non-random data structure.
- Add item-listing-search.graphql schema definition - Add azure-vs-mock-comparison.ts example for cognitive search - Clean up remaining untracked files from development
- Add dist/ folders to .gitignore - Add *.tsbuildinfo to .gitignore - Add test-*.js files to .gitignore - Add *.json test results to .gitignore
- Add LunrSearchEngine wrapper with TF-IDF relevance scoring - Implement field boosting (title 10x, description 2x) - Add wildcard and fuzzy matching support - Include comprehensive test suite (20 tests) - Add JSDoc documentation following CellixJS standards - Maintain Azure Cognitive Search API compatibility - Fix query processing and consistent count handling
- Add 27 tests for AzureCognitiveSearch implementation - Constructor and authentication (API key + DefaultAzureCredential) - Service lifecycle (startup/shutdown) - Index management (create, delete, exists) - Document operations (index, delete with proper error handling) - Search operations with facets and filtering - Field type conversion (all Edm types) - Field attribute handling (searchable, filterable, etc.) - Add 12 tests for ServiceCognitiveSearch wrapper - Environment detection (USE_MOCK_SEARCH, USE_AZURE_SEARCH) - Service lifecycle - Proxy method delegation - Graceful fallback handling - Add comprehensive test documentation - TESTING_README.md: Quick start guide and commands - TEST_DOCUMENTATION.md: Detailed test descriptions (850+ lines) - TEST_EXECUTION_SUMMARY.md: Execution summary and coverage - Configure vitest for test execution - vitest.config.ts with coverage settings - Proper vi.hoisted() pattern for mock declarations - Type-safe imports from @cellix/mock-cognitive-search Test Results: 39/39 passing (100% success rate) Mock Coverage: Full Azure SDK isolation (@azure/search-documents, @azure/identity) Type Safety: All TypeScript errors resolved with proper type assertions Note: Identified bug in deleteDocument implementation (documented in test)
6cf862f to
dd1dd04
Compare
- Resolved conflicts in package.json files (migrated to pnpm) - Resolved conflicts in apps/api/src/index.ts (kept cognitive search service) - Resolved conflicts in .gitignore (combined build artifacts) - Removed deleted files (package-lock.json, tsconfig.tsbuildinfo) - Updated workspace dependencies to use workspace:* syntax - Preserved cognitive search functionality while adopting main's changes
…add safety-cap and linear parser; add ambient types for `liqe` and narrow parsed.type access
This reverts commit e76a978.
…ix/* deps and refresh pnpm-lock.yaml
…CI (use --with-deps and cache path)
…listing to resolve facet type conflict
…y to satisfy type imports
…rch to @sthrift/domain
- Add searchItemListings GraphQL query document with full field selection - Update listings-page.container to use GraphQL search instead of client-side filtering - Implement conditional query logic: use search when filtering/searching, otherwise show all - Hero section search now triggers backend search via searchItemListings query - Support pagination and category filtering through GraphQL search API - Maintains backward compatibility for showing all listings when no filters applied
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 99 out of 107 changed files in this pull request and generated no new comments.
- Split search state into searchInputValue (UI) and searchQuery (API trigger) - Search now executes only on Enter key press or button click - Added onKeyDown handler to SearchBar component for Enter key support - Prevents unnecessary network requests and improves UX - Fixes issue where typing single letters would trigger failed searches
- Simplified SearchBar onSearch callback to not pass query parameter - Parent components now use searchInputValue state directly - Both Enter key and search button now correctly trigger search - Updated HeroSection and ListingsPage components to match new API
- Add 500ms debounce to search input for better UX - Search executes automatically after user stops typing - Enter key or search button triggers immediate search (bypasses debounce) - Prevents excessive API calls while maintaining responsive search - Uses useEffect with cleanup for proper timer management
- Create useDebouncedValue hook following React best practices - Move debounce logic out of component into dedicated hook file - Add triggerImmediate function for explicit search (Enter/button click) - Aligns with codebase pattern of extracting hooks into separate files - Follows existing hook patterns (use-create-listing-navigation, use-file-limit) - Cleaner, more testable, and reusable implementation
- Remove setCurrentPage from handleSearchChange to prevent lockout on typing - Use useEffect to reset page only when debounced value changes - Fix triggerImmediate closure issue using useRef and useCallback - Add valueRef to capture current value for immediate trigger - Enter key and search button now work correctly - No more typing interference from state updates
- Remove debounced search that was causing typing interference - Search now only executes on Enter key press or search button click - Separate searchInputValue (UI state) from searchQuery (API trigger) - User can type freely without any lockout or interference - Simpler, more predictable user experience
Resolved conflicts: - item-listing.resolvers.feature: Kept feature branch's domain state values (Published, Drafted, etc.) - item-listing.resolvers.test.ts: Kept feature branch's queryPagedWithSearchFallback implementation - pnpm-lock.yaml: Regenerated after merge to ensure consistency
- Fix test scenario to use 'Active' status filter instead of 'Published' to match feature spec - Remove unused use-debounced-value.ts hook file (detected by Knip audit)
…ature spec The test was checking for 'publishedAt' field but the feature file specifies 'createdAt'. This caused the Azure Pipeline build to fail. Updated the interface, mapping, and assertion to use the correct field name.
- Renamed GraphQL types: ItemListingSearchInput → ListingSearchInput, searchItemListings → searchListings, bulkIndexItemListings → bulkIndexListings - Updated GraphQL schema and resolvers to use generic Listing naming - Renamed domain interfaces: ItemListingSearchDocument → ListingSearchDocument, ItemListingSearchResult → ListingSearchResult, ItemListingSearchFilter → ListingSearchFilter - Created new search index files: listing-search-index.ts with ListingSearchIndexSpec - Created new service files: listing-search-indexing.ts and listing-search.ts - Updated application service layer: ItemListingSearchApplicationService → ListingSearchApplicationService - Updated UI GraphQL queries and component references - Regenerated GraphQL types with codegen - Deleted old files: item-listing-search-index.ts, item-listing-search-indexing.ts, item-listing-search.ts - Changed index name from 'item-listings' to 'listings' - Changed env var from SEARCH_ITEM_LISTING_INDEX_NAME to SEARCH_LISTING_INDEX_NAME This makes the search functionality more generic and reusable for any listing type, following the pattern suggested in PR review.
Complete the refactoring by updating all remaining references: - Update service-search-index.ts and its test to use ListingSearchIndexSpec - Fix domain package export to use ListingSearchIndexingService - Update event handlers to use new service and method names: - ListingSearchIndexingService (was ItemListingSearchIndexingService) - indexListing() (was indexItemListing()) - Update application service query-paged-with-search.ts These changes were missed in the initial refactoring commit and were causing Azure Pipeline build failures due to unresolved references.
- Replace 'item-listings' with 'listings' in query-paged-with-search.test.ts (8 occurrences) - Update context factory test to expect ListingSearch instead of ItemListingSearch - All 895 application-services tests now pass
- Add 83 tests covering all fields in ListingSearchIndexSpec - Test field properties (searchable, filterable, sortable, facetable, retrievable) - Test convertListingToSearchDocument with various scenarios - Handle missing data, nested structures, edge cases - Achieve 100% code coverage for listing-search-index.ts
- Import and use convertListingToSearchDocument from domain - Replaces manual field mapping with proper converter - Ensures consistent data transformation between indexing and search - Fixes issue where sharerName only included firstName
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
Copilot reviewed 101 out of 109 changed files in this pull request and generated no new comments.
Files not reviewed (1)
- pnpm-lock.yaml: Language not supported
- Access raw props directly instead of domain entity getters to avoid crashes - Add graceful fallback for missing nested data (sharer.account.profile) - Extract value objects properly (e.g., description.value) - Populate sharer field in bulkIndexListings query - Add detailed logging for troubleshooting indexing issues Fixes issue where bulkIndexListings failed with 'Cannot read properties of undefined' errors when domain entities had unpopulated nested fields. Result: Successfully indexes 10/10 listings with proper error handling.
- Implement automatic search indexing via integration events - ItemListing.onSave() raises ItemListingUpdatedEvent and ItemListingDeletedEvent - Event handlers automatically update search index on create/update/delete - No manual reindexing needed during runtime - Fix architectural duplication (clean architecture) - Remove duplicate ListingSearchIndexSpec from search-service-index - Make infrastructure layer import from domain layer - Establish domain as single source of truth for business logic - Fix converter to handle incomplete domain entities - Access raw props to avoid undefined nested properties - Handle both domain entities and test objects - Extract values from value objects properly - Clean up diagnostic logging - Remove console.log statements from production code - Keep only error logging for troubleshooting - Fix build configuration - Add runtime export to types-only packages - Switch to node.json base config for correct output paths - Fix package.json exports to point to dist/src/ - Add TypeScript project references - Add comprehensive documentation - documents/automatic-search-indexing.md: Architecture and usage guide - documents/test-automatic-indexing.md: Step-by-step testing scenarios All 2924 tests passing. All 29 packages build successfully.
The test was still importing from the deleted indexes/ directory. Now imports ListingSearchIndexSpec from @sthrift/domain instead.
…vice - 30 test cases covering all methods and edge cases - Tests for searchListings with various filters (category, state, location, date range, sharerId) - Tests for pagination, sorting, and facets - Tests for bulkIndexListings including error handling - Tests for edge cases (null values, empty arrays, etc.) - Achieves 100% code coverage for listing-search.ts
- listing-search.feature: 33 scenarios covering search, filtering, pagination, bulk indexing - service-search-index.feature: 31 scenarios covering facade operations, CRUD, search - item-listing.feature: Added 5 scenarios for onSave() integration event behavior Feature files document behavior in Gherkin format for better understanding and alignment with existing codebase documentation patterns.
… scenarios - Implemented 5 vitest-cucumber scenario tests for ItemListing onSave() behavior - Scenario: Raising integration event when listing is modified - Scenario: Not raising update event when listing is not modified - Scenario: Raising integration event when listing is deleted - Scenario: Prioritizing delete event over update event - Uses instanceof checks for ItemListingUpdatedEvent and ItemListingDeletedEvent - All 257 tests passing (22ms) Fixes Azure Pipeline ScenarioNotCalledError for domain tests
…tory The ListingsPageContainer now uses both itemListings and searchListings GraphQL queries depending on whether filters/search are active. The Storybook stories were only mocking the itemListings query, causing Apollo MockLink to fail with module loading errors in CI. Root cause: - Container uses conditional query logic (shouldUseSearch flag) - When no search/filter: queries itemListings - When searching/filtering: queries searchListings - Story only mocked itemListings, causing unmocked query error Changes: - Import ListingsPageSearchListingsDocument from generated types - Add searchListings query mock to meta.parameters.apolloClient.mocks - Include proper GraphQL variables structure (searchString, options, filter) - Add __typename fields for Apollo cache normalization - Include sharerName/sharerId fields required by search document - Add facets mock data (category, state, sharerId) - Update EmptyListings story with empty search results mock - Remove unnecessary async from Loading story play function Follows codebase patterns: - Multiple query mocking (same as reservations-view-active.stories) - Proper __typename usage for cache normalization - Co-located with container and .graphql files - Uses withMockApolloClient decorator pattern Fixes CI test failures: - Test Files: 1 failed | 100 passed (101) - Tests: 5 failed | 552 passed (557) - Error: 'Failed to fetch dynamically imported module' All backend tests continue passing: - Domain: 3083 tests ✓ - Application Services: 932 tests ✓
Description
Overview
This PR implements the end-to-end Cognitive Search infrastructure and associated developer experience improvements. It adds an environment-aware search service that can switch between Azure Cognitive Search and a local mock implementation, introduces a LiQE-based filter engine, strengthens CI reliability (pnpm lockfile, Playwright browsers), and fixes Storybook states for Reservations.
What’s included
ServiceCognitiveSearchthat selects Azure or Mock at runtime via env vars@sthrift/service-cognitive-searchintegration with@cellix/api-services-specliqeambient types and safe narrowingComponentQueryLoader.errorComponent@cellix/*deps toworkspace:*, refreshedpnpm-lock.yamlpnpm dlx playwright@latest install --with-depsConfiguration
SEARCH_SERVICE_MODE=azureAZURE_SEARCH_ENDPOINT=...AZURE_SEARCH_API_KEY=...SEARCH_SERVICE_MODE=mockVerification
Security/Performance
Breaking changes
Notes
Summary by Sourcery
Implement end-to-end Cognitive Search infrastructure with environment-based switching between Azure and in-memory mock, integrate search into application services and GraphQL API, and improve developer experience and CI reliability.
New Features:
Enhancements:
Build:
Tests: