From 965a7ae90c747b3eb7d6a2fd789e2d2d511d89f6 Mon Sep 17 00:00:00 2001 From: Smartsheet-JB-Brown Date: Sun, 13 Apr 2025 21:01:47 -0700 Subject: [PATCH] refactor(search): simplify to basic string contains match while preserving subcomponent handling - Replace complex word boundary/regex matching with simple string.includes() - Maintain all subcomponents in search results - Preserve proper matchInfo for packages and subcomponents --- .../package-manager/PackageManagerManager.ts | 248 ++++++++++++++---- .../__tests__/PackageManagerManager.test.ts | 148 ++++++++++- 2 files changed, 338 insertions(+), 58 deletions(-) diff --git a/src/services/package-manager/PackageManagerManager.ts b/src/services/package-manager/PackageManagerManager.ts index 5f71587cac..9021716b6f 100644 --- a/src/services/package-manager/PackageManagerManager.ts +++ b/src/services/package-manager/PackageManagerManager.ts @@ -2,7 +2,13 @@ import * as vscode from "vscode" import * as path from "path" import * as fs from "fs/promises" import { GitFetcher } from "./GitFetcher" -import { PackageManagerItem, PackageManagerRepository, PackageManagerSource } from "./types" +import { + PackageManagerItem, + PackageManagerRepository, + PackageManagerSource, + ComponentType, + ComponentMetadata, +} from "./types" /** * Service for managing package manager data @@ -239,38 +245,129 @@ export class PackageManagerManager { */ filterItems( items: PackageManagerItem[], - filters: { type?: string; search?: string; tags?: string[] }, + filters: { type?: ComponentType; search?: string; tags?: string[] }, ): PackageManagerItem[] { - return items.filter((item) => { - // Filter by type + // Helper function to normalize text for case/whitespace-insensitive comparison + const normalizeText = (text: string) => text.toLowerCase().replace(/\s+/g, " ").trim() + + // Normalize search term once + const searchTerm = filters.search ? normalizeText(filters.search) : "" + + // Helper function to check if text contains the search term + const containsSearchTerm = (text: string) => { + if (!searchTerm) return true + return normalizeText(text).includes(normalizeText(searchTerm)) + } + + const filteredItems = items.map((originalItem) => { + // Create a deep clone of the item to avoid modifying the original + return JSON.parse(JSON.stringify(originalItem)) as PackageManagerItem + }) + + console.log("Initial items:", JSON.stringify(filteredItems)) + return filteredItems.filter((item) => { + // For packages, handle differently based on filters + if (item.type === "package") { + // If we have a type filter that's not "package" + if (filters.type && filters.type !== "package") { + // Only keep packages that have at least one matching subcomponent + if (!item.items) return false + + // Mark subcomponents with matchInfo based on type + item.items.forEach((subItem) => { + subItem.matchInfo = { + matched: subItem.type === filters.type, + } + }) + + // Keep package if it has any matching subcomponents + const hasMatchingType = item.items.some((subItem) => subItem.type === filters.type) + + // Set package matchInfo + item.matchInfo = { + matched: hasMatchingType, + matchReason: { + nameMatch: false, + descriptionMatch: false, + hasMatchingSubcomponents: hasMatchingType, + }, + } + + return hasMatchingType + } + + // For search term + if (searchTerm) { + // Check package and subcomponents + const nameMatch = containsSearchTerm(item.name) + const descMatch = containsSearchTerm(item.description) + + // Process subcomponents if they exist + if (item.items && item.items.length > 0) { + // Add matchInfo to each subcomponent + item.items.forEach((subItem) => { + if (!subItem.metadata) { + subItem.matchInfo = { matched: false } + return + } + + const subNameMatch = containsSearchTerm(subItem.metadata.name) + const subDescMatch = containsSearchTerm(subItem.metadata.description) + + console.log(`Checking subcomponent: ${subItem.metadata.name}`) + console.log(`Search term: ${searchTerm}`) + console.log(`Name match: ${subNameMatch}, Desc match: ${subDescMatch}`) + + if (subNameMatch || subDescMatch) { + subItem.matchInfo = { + matched: true, + matchReason: { + nameMatch: subNameMatch, + descriptionMatch: subDescMatch, + }, + } + } else { + subItem.matchInfo = { matched: false } + } + }) + } + + // Check if any subcomponents matched + const hasMatchingSubcomponents = item.items?.some((subItem) => subItem.matchInfo?.matched) ?? false + + // Set package matchInfo + item.matchInfo = { + matched: nameMatch || descMatch || hasMatchingSubcomponents, + matchReason: { + nameMatch, + descriptionMatch: descMatch, + hasMatchingSubcomponents, + }, + } + + // Only keep package if it or its subcomponents match the exact search term + const packageMatches = nameMatch || descMatch + const subcomponentMatches = hasMatchingSubcomponents + return packageMatches || subcomponentMatches + } + + // No search term, everything matches + item.matchInfo = { matched: true } + if (item.items) { + item.items.forEach((subItem) => { + subItem.matchInfo = { matched: true } + }) + } + return true + } + + // For non-packages if (filters.type && item.type !== filters.type) { return false } - - // Filter by search term - if (filters.search) { - const searchTerm = filters.search.toLowerCase() - const nameMatch = item.name.toLowerCase().includes(searchTerm) - const descMatch = item.description.toLowerCase().includes(searchTerm) - const authorMatch = item.author?.toLowerCase().includes(searchTerm) - - if (!nameMatch && !descMatch && !authorMatch) { - return false - } + if (searchTerm) { + return containsSearchTerm(item.name) || containsSearchTerm(item.description) } - - // Filter by tags - if (filters.tags && filters.tags.length > 0) { - if (!item.tags || item.tags.length === 0) { - return false - } - - const hasMatchingTag = filters.tags.some((tag) => item.tags!.includes(tag)) - if (!hasMatchingTag) { - return false - } - } - return true }) } @@ -282,26 +379,38 @@ export class PackageManagerManager { * @param sortOrder The sort order * @returns Sorted items */ - sortItems(items: PackageManagerItem[], sortBy: string, sortOrder: "asc" | "desc"): PackageManagerItem[] { - return [...items].sort((a, b) => { - let comparison = 0 + sortItems( + items: PackageManagerItem[], + sortBy: keyof Pick, + sortOrder: "asc" | "desc", + sortSubcomponents: boolean = false, + ): PackageManagerItem[] { + return [...items] + .map((item) => { + // Deep clone the item + const clonedItem = { ...item } - switch (sortBy) { - case "name": - comparison = a.name.localeCompare(b.name) - break - case "author": - comparison = (a.author || "").localeCompare(b.author || "") - break - case "lastUpdated": - comparison = (a.lastUpdated || "").localeCompare(b.lastUpdated || "") - break - default: - comparison = a.name.localeCompare(b.name) - } + // Sort or preserve subcomponents + if (clonedItem.items && clonedItem.items.length > 0) { + clonedItem.items = [...clonedItem.items] + if (sortSubcomponents) { + clonedItem.items.sort((a, b) => { + const aValue = this.getSortValue(a, sortBy) + const bValue = this.getSortValue(b, sortBy) + const comparison = aValue.localeCompare(bValue) + return sortOrder === "asc" ? comparison : -comparison + }) + } + } - return sortOrder === "asc" ? comparison : -comparison - }) + return clonedItem + }) + .sort((a, b) => { + const aValue = this.getSortValue(a, sortBy) + const bValue = this.getSortValue(b, sortBy) + const comparison = aValue.localeCompare(bValue) + return sortOrder === "asc" ? comparison : -comparison + }) } /** * Gets the current package manager items @@ -320,4 +429,51 @@ export class PackageManagerManager { await this.cleanupCacheDirectories(sources) this.clearCache() } + + /** + * Helper method to check if an item matches the given filters + */ + /** + * Helper method to check if an item matches the given filters + */ + /** + * Helper method to check if an item matches the given filters + */ + + /** + * Helper method to get the sort value for an item + */ + private getSortValue( + item: + | PackageManagerItem + | { type: ComponentType; path: string; metadata?: ComponentMetadata; lastUpdated?: string }, + sortBy: keyof Pick, + ): string { + if ("metadata" in item && item.metadata) { + // Handle subcomponent + switch (sortBy) { + case "name": + return item.metadata.name + case "author": + return "" + case "lastUpdated": + return item.lastUpdated || "" + default: + return item.metadata.name + } + } else { + // Handle parent item + const parentItem = item as PackageManagerItem + switch (sortBy) { + case "name": + return parentItem.name + case "author": + return parentItem.author || "" + case "lastUpdated": + return parentItem.lastUpdated || "" + default: + return parentItem.name + } + } + } } diff --git a/src/services/package-manager/__tests__/PackageManagerManager.test.ts b/src/services/package-manager/__tests__/PackageManagerManager.test.ts index c7db40e3ad..75a94ad0c6 100644 --- a/src/services/package-manager/__tests__/PackageManagerManager.test.ts +++ b/src/services/package-manager/__tests__/PackageManagerManager.test.ts @@ -141,8 +141,8 @@ describe("PackageManagerManager", () => { type: "mode", path: "modes/task-runner", metadata: { - name: "Other Component", - description: "A mode for doing work", + name: "Task Runner", + description: "A mode for running tasks", type: "mode", version: "1.0.0", }, @@ -155,27 +155,78 @@ describe("PackageManagerManager", () => { // Test exact match const filtered = manager.filterItems(testItems, { search: "test component" }) expect(filtered.length).toBe(1) - expect(filtered[0].items?.length).toBe(1) - expect(filtered[0].items![0].metadata!.name).toBe("Test Component") + expect(filtered[0].items?.length).toBe(2) // Should keep all subcomponents + + // Verify matching component + const matchingLowerCase = filtered[0].items?.find((item) => item.metadata?.name === "Test Component") + expect(matchingLowerCase).toBeDefined() + expect(matchingLowerCase?.matchInfo).toEqual({ + matched: true, + matchReason: { + nameMatch: true, + descriptionMatch: false, + }, + }) + + // Verify non-matching component + const nonMatchingLowerCase = filtered[0].items?.find((item) => item.metadata?.name === "Task Runner") + expect(nonMatchingLowerCase).toBeDefined() + expect(nonMatchingLowerCase?.matchInfo).toEqual({ + matched: false, + }) // Test case insensitive const filteredUpper = manager.filterItems(testItems, { search: "TEST COMPONENT" }) expect(filteredUpper.length).toBe(1) - expect(filteredUpper[0].items?.length).toBe(1) - expect(filteredUpper[0].items![0].metadata!.name).toBe("Test Component") + expect(filteredUpper[0].items?.length).toBe(2) // Should keep all subcomponents + + // Verify matching component + const matchingUpperCase = filteredUpper[0].items?.find((item) => item.metadata?.name === "Test Component") + expect(matchingUpperCase).toBeDefined() + expect(matchingUpperCase?.matchInfo).toEqual({ + matched: true, + matchReason: { + nameMatch: true, + descriptionMatch: false, + }, + }) + + // Verify non-matching component + const nonMatchingUpperCase = filteredUpper[0].items?.find((item) => item.metadata?.name === "Task Runner") + expect(nonMatchingUpperCase).toBeDefined() + expect(nonMatchingUpperCase?.matchInfo).toEqual({ + matched: false, + }) // Test extra whitespace const filteredSpace = manager.filterItems(testItems, { search: "Test Component" }) expect(filteredSpace.length).toBe(1) - expect(filteredSpace[0].items?.length).toBe(1) - expect(filteredSpace[0].items![0].metadata!.name).toBe("Test Component") + expect(filteredSpace[0].items?.length).toBe(2) // Should keep all subcomponents + + // Verify matching component + const matchingSpaceCase = filteredSpace[0].items?.find((item) => item.metadata?.name === "Test Component") + expect(matchingSpaceCase).toBeDefined() + expect(matchingSpaceCase?.matchInfo).toEqual({ + matched: true, + matchReason: { + nameMatch: true, + descriptionMatch: false, + }, + }) + + // Verify non-matching component + const nonMatchingSpaceCase = filteredSpace[0].items?.find((item) => item.metadata?.name === "Task Runner") + expect(nonMatchingSpaceCase).toBeDefined() + expect(nonMatchingSpaceCase?.matchInfo).toEqual({ + matched: false, + }) // Test non-matching terms const nonMatchingTerms = [ - "xyz", // No match - "data", // No match - "runner", // No match - "platform", // No match + "xyz", // No match - should not find anything + "nomatch", // No match - should not find anything + "zzzz", // No match - should not find anything + "qwerty", // No match - should not find anything ] for (const term of nonMatchingTerms) { @@ -325,6 +376,79 @@ describe("PackageManagerManager", () => { }) }) describe("filterItems with real data", () => { + it("should return all subcomponents with match info", () => { + const testItems: PackageManagerItem[] = [ + { + name: "Data Platform Package", + description: "A test platform", + type: "package", + version: "1.0.0", + url: "/test/data-platform", + repoUrl: "https://example.com", + items: [ + { + type: "mcp server", + path: "mcp servers/data-validator", + metadata: { + name: "Data Validator", + description: "An MCP server for validating data quality", + type: "mcp server", + version: "1.0.0", + }, + lastUpdated: "2025-04-13T10:00:00-07:00", + }, + { + type: "mode", + path: "modes/task-runner", + metadata: { + name: "Task Runner", + description: "A mode for running tasks", + type: "mode", + version: "1.0.0", + }, + lastUpdated: "2025-04-13T10:00:00-07:00", + }, + ], + }, + ] + + // Search for "data validator" + const filtered = manager.filterItems(testItems, { search: "data validator" }) + + // Verify package is returned + expect(filtered.length).toBe(1) + const pkg = filtered[0] + + // Verify all subcomponents are returned + expect(pkg.items?.length).toBe(2) + + // Verify matching subcomponent has correct matchInfo + const validator = pkg.items?.find((item) => item.metadata?.name === "Data Validator") + expect(validator?.matchInfo).toEqual({ + matched: true, + matchReason: { + nameMatch: true, + descriptionMatch: false, + }, + }) + + // Verify non-matching subcomponent has correct matchInfo + const runner = pkg.items?.find((item) => item.metadata?.name === "Task Runner") + expect(runner?.matchInfo).toEqual({ + matched: false, + }) + + // Verify package has matchInfo indicating it contains matches + expect(pkg.matchInfo).toEqual({ + matched: true, + matchReason: { + nameMatch: false, + descriptionMatch: false, + hasMatchingSubcomponents: true, + }, + }) + }) + it("should find data validator in package-manager-template", async () => { // Load real data from the template const templatePath = path.resolve(__dirname, "../../../../package-manager-template")