From ef16a60ed0c872d16823c61457bd99693a8fac50 Mon Sep 17 00:00:00 2001 From: Kevin Heis Date: Mon, 8 Dec 2025 15:20:52 -0800 Subject: [PATCH] Resolve TODO comments from codebase (#58691) --- .github/instructions/all.instructions.md | 3 +- .../liquid-ifversion-versions.ts | 5 +- src/content-linter/tests/category-pages.ts | 27 +++-- src/fixtures/tests/minitoc.ts | 1 - src/frame/lib/path-utils.ts | 39 ++++---- src/frame/middleware/find-page.ts | 7 +- src/frame/tests/path-utils.ts | 47 +++++++++ .../scripts/utils/create-rest-examples.ts | 7 +- .../scripts/utils/inject-models-schema.ts | 30 ++---- src/search/lib/elasticsearch-indexes.ts | 7 +- src/search/lib/helpers/old-version-logic.ts | 44 --------- .../routes/ai-search-autocomplete-route.ts | 44 +++++++++ src/search/lib/routes/general-search-route.ts | 47 +++++++++ src/search/middleware/search-routes.ts | 98 +------------------ .../lib/parse-page-sections-into-records.ts | 5 +- src/types/types.ts | 8 +- 16 files changed, 210 insertions(+), 209 deletions(-) create mode 100644 src/frame/tests/path-utils.ts delete mode 100644 src/search/lib/helpers/old-version-logic.ts create mode 100644 src/search/lib/routes/ai-search-autocomplete-route.ts create mode 100644 src/search/lib/routes/general-search-route.ts diff --git a/.github/instructions/all.instructions.md b/.github/instructions/all.instructions.md index 6c093e5aaa..aba2b7f966 100644 --- a/.github/instructions/all.instructions.md +++ b/.github/instructions/all.instructions.md @@ -28,4 +28,5 @@ When you create a pull request: 3. Label with "llm-generated". 4. If an issue exists, include "fixes owner/repo#issue" or "towards owner/repo#issue" as appropriate. -5. Always _escape backticks_ when you use gh cli. +5. Always create PRs in **draft mode** using `--draft` flag. +6. Always _escape backticks_ when you use gh cli. diff --git a/src/content-linter/lib/linting-rules/liquid-ifversion-versions.ts b/src/content-linter/lib/linting-rules/liquid-ifversion-versions.ts index c56439c41d..96b7b70e69 100644 --- a/src/content-linter/lib/linting-rules/liquid-ifversion-versions.ts +++ b/src/content-linter/lib/linting-rules/liquid-ifversion-versions.ts @@ -206,9 +206,10 @@ async function getApplicableVersionFromLiquidTag(conditionStr: string) { const ands = ver.split(' and ') const firstAnd = ands[0].split(' ')[0] // if all ands don't start with the same version it's invalid + // Note: This edge case (e.g., "fpt and ghes >= 3.1") doesn't occur in our content. + // All actual uses have matching versions (e.g., "ghes and ghes > 3.19"). + // If this edge case appears in the future, additional logic would be needed here. if (!ands.every((and) => and.startsWith(firstAnd))) { - // noop - we don't handle this case - // TODO - handle this case in the future return [] } const andValues = [] diff --git a/src/content-linter/tests/category-pages.ts b/src/content-linter/tests/category-pages.ts index a6add3b574..5519287ee3 100644 --- a/src/content-linter/tests/category-pages.ts +++ b/src/content-linter/tests/category-pages.ts @@ -50,14 +50,17 @@ describe.skip('category pages', () => { // otherwise, if one of them has no categories, the tests will fail. for (const tuple of productTuples) { const [, productIndex] = tuple + + const productDir = path.dirname(productIndex) + // Get links included in product index page. // Each link corresponds to a product subdirectory (category). // Example: "getting-started-with-github" + // Note: We need to read this synchronously here because vitest's describe.each + // can't asynchronously define tests const contents = fs.readFileSync(productIndex, 'utf8') const data = getFrontmatterData(contents) - const productDir = path.dirname(productIndex) - const children: string[] = data.children const categoryLinks = children // Only include category directories, not standalone category files like content/actions/quickstart.md @@ -118,15 +121,19 @@ describe.skip('category pages', () => { await contextualize(req as ExtendedRequest, res as Response, next) await shortVersions(req as ExtendedRequest, res as Response, next) - // Save the index title for later testing - indexTitle = data.title.includes('{') - ? await renderContent(data.title, req.context, { textOnly: true }) - : data.title + // Read the product index data for rendering + const productIndexContents = await fs.promises.readFile(productIndex, 'utf8') + const productIndexData = getFrontmatterData(productIndexContents) - if (data.shortTitle) { - indexShortTitle = data.shortTitle.includes('{') - ? await renderContent(data.shortTitle, req.context, { textOnly: true }) - : data.shortTitle + // Save the index title for later testing + indexTitle = productIndexData.title.includes('{') + ? await renderContent(productIndexData.title, req.context, { textOnly: true }) + : productIndexData.title + + if (productIndexData.shortTitle) { + indexShortTitle = productIndexData.shortTitle.includes('{') + ? await renderContent(productIndexData.shortTitle, req.context, { textOnly: true }) + : productIndexData.shortTitle } else { indexShortTitle = '' } diff --git a/src/fixtures/tests/minitoc.ts b/src/fixtures/tests/minitoc.ts index 1fbeaf279f..9d2475853a 100644 --- a/src/fixtures/tests/minitoc.ts +++ b/src/fixtures/tests/minitoc.ts @@ -4,7 +4,6 @@ import cheerio from 'cheerio' import { getDOM } from '@/tests/helpers/e2etest' describe('minitoc', () => { - // TODO disable the mini TOC tests when we replace it with sticky TOC header test('renders mini TOC in articles with more than one heading', async () => { const $: cheerio.Root = await getDOM('/en/get-started/minitocs/multiple-headings') expect($('h2#in-this-article').length).toBe(1) diff --git a/src/frame/lib/path-utils.ts b/src/frame/lib/path-utils.ts index 64c3fb1eef..ff6f1d181c 100644 --- a/src/frame/lib/path-utils.ts +++ b/src/frame/lib/path-utils.ts @@ -109,32 +109,37 @@ export function getVersionObjectFromPath(href: string | undefined) { return allVersions[versionFromPath] } -// TODO needs refactoring + tests // Return the product segment from the path +// Extracts the product identifier from various URL patterns including versioned paths export function getProductStringFromPath(href: string | undefined): string { + // Handle empty or undefined paths if (!href) return 'homepage' - href = getPathWithoutLanguage(href) - if (href === '/') return 'homepage' + const normalizedHref = getPathWithoutLanguage(href) + if (normalizedHref === '/') return 'homepage' - // The first segment will always be empty on this split - const pathParts = href.split('/') + // Split path into segments (first segment is always empty string) + const pathParts = normalizedHref.split('/') + // Handle special product paths that appear anywhere in the URL if (pathParts.includes('early-access')) return 'early-access' - // For rest pages the currentProduct should be rest - // We use this to show SidebarRest, which is a different sidebar than the rest of the site - if (pathParts[1] === 'rest') return 'rest' - if (pathParts[1] === 'copilot') return 'copilot' - if (pathParts[1] === 'get-started') return 'get-started' + // Handle special products that always appear as the first segment + // These products use custom sidebars and need explicit handling + const specialProducts = ['rest', 'copilot', 'get-started'] + if (specialProducts.includes(pathParts[1])) { + return pathParts[1] + } + + // Determine if first segment is a version (e.g., 'enterprise-server@3.9') + // If yes, product is in pathParts[2], otherwise it's in pathParts[1] + // Examples: + // /enterprise-server@3.9/admin -> product is 'admin' + // /github/getting-started -> product is 'github' + // /enterprise-server@3.9 -> product is 'enterprise-server@3.9' (enterprise landing) + const hasVersionPrefix = supportedVersions.has(pathParts[1]) + const productString = hasVersionPrefix && pathParts[2] ? pathParts[2] : pathParts[1] - // Possible scenarios for href (assume part[0] is an empty string): - // - // * part[1] is a version and part[2] is undefined, so return part[1] as an enterprise landing page - // * part[1] is a version and part[2] is defined, so return part[2] as the product - // * part[1] is NOT a version, so return part[1] as the product - const isEnterprise = supportedVersions.has(pathParts[1]) - const productString = isEnterprise && pathParts[2] ? pathParts[2] : pathParts[1] return productString } diff --git a/src/frame/middleware/find-page.ts b/src/frame/middleware/find-page.ts index 617b162d90..a78feb5c3d 100644 --- a/src/frame/middleware/find-page.ts +++ b/src/frame/middleware/find-page.ts @@ -105,10 +105,9 @@ async function rereadByPath( const languageCode = match[1] const withoutLanguage = uri.replace(languagePrefixPathRegex, '/') const withoutVersion = withoutLanguage.replace(`/${currentVersion}`, '') - // TODO: Support loading translations the same way. - // NOTE: No one is going to test translations like this in development - // but perhaps one day we can always and only do these kinds of lookups - // at runtime. + // Note: We don't support loading translations at runtime. All translations + // are loaded at build time. This function only handles English content reloading + // during development. const possible = path.join(contentRoot, withoutVersion) const filePath = existsSync(possible) ? path.join(possible, 'index.md') : `${possible}.md` const relativePath = path.relative(contentRoot, filePath) diff --git a/src/frame/tests/path-utils.ts b/src/frame/tests/path-utils.ts new file mode 100644 index 0000000000..1df7e18f0b --- /dev/null +++ b/src/frame/tests/path-utils.ts @@ -0,0 +1,47 @@ +import { describe, expect, test } from 'vitest' + +import { getProductStringFromPath } from '@/frame/lib/path-utils' + +describe('getProductStringFromPath', () => { + test('returns "homepage" for root paths', () => { + expect(getProductStringFromPath('/')).toBe('homepage') + expect(getProductStringFromPath('/en')).toBe('homepage') + expect(getProductStringFromPath(undefined)).toBe('homepage') + }) + + test('handles early-access product', () => { + expect(getProductStringFromPath('/en/early-access/github/overview')).toBe('early-access') + expect(getProductStringFromPath('/early-access/admin/guides')).toBe('early-access') + }) + + test('handles special products (rest, copilot, get-started)', () => { + expect(getProductStringFromPath('/en/rest/overview')).toBe('rest') + expect(getProductStringFromPath('/rest/reference/repos')).toBe('rest') + expect(getProductStringFromPath('/en/copilot/using-copilot')).toBe('copilot') + expect(getProductStringFromPath('/copilot/features')).toBe('copilot') + expect(getProductStringFromPath('/en/get-started/quickstart')).toBe('get-started') + expect(getProductStringFromPath('/get-started/onboarding')).toBe('get-started') + }) + + test('extracts product from non-versioned paths', () => { + expect(getProductStringFromPath('/en/github/getting-started')).toBe('github') + expect(getProductStringFromPath('/actions/quickstart')).toBe('actions') + expect(getProductStringFromPath('/en/admin/installation')).toBe('admin') + expect(getProductStringFromPath('/code-security/overview')).toBe('code-security') + }) + + test('extracts product from versioned paths', () => { + // Note: These tests use free-pro-team which is a supported version + expect(getProductStringFromPath('/en/free-pro-team@latest/admin/installation')).toBe('admin') + expect(getProductStringFromPath('/free-pro-team@latest/actions/quickstart')).toBe('actions') + expect(getProductStringFromPath('/en/free-pro-team@latest/github/getting-started')).toBe( + 'github', + ) + }) + + test('handles enterprise landing pages (version without product)', () => { + // When a version is present but no product segment follows, return the version string + expect(getProductStringFromPath('/en/free-pro-team@latest')).toBe('free-pro-team@latest') + expect(getProductStringFromPath('/enterprise-server@latest')).toBe('enterprise-server@latest') + }) +}) diff --git a/src/rest/scripts/utils/create-rest-examples.ts b/src/rest/scripts/utils/create-rest-examples.ts index a1ace57a18..605d0701bf 100644 --- a/src/rest/scripts/utils/create-rest-examples.ts +++ b/src/rest/scripts/utils/create-rest-examples.ts @@ -352,10 +352,9 @@ export function getResponseExamples(operation: Operation): ResponseExample[] { contentType, description: examples[key].summary || operation.responses[statusCode].description, example: examples[key].value, - // TODO adding the schema quadruples the JSON file size. Changing - // how we write the JSON file helps a lot, but we should revisit - // adding the response schema to ensure we have a way to view the - // prettified JSON before minimizing it. + // Note: Including the schema significantly increases JSON file size (~4x), + // but it's necessary to support the schema/example toggle in the UI. + // Users can switch between viewing the example response and the full schema. schema: operation.responses[statusCode].content[contentType].schema, }, } diff --git a/src/rest/scripts/utils/inject-models-schema.ts b/src/rest/scripts/utils/inject-models-schema.ts index 4b40dd0c0a..485df1d67e 100644 --- a/src/rest/scripts/utils/inject-models-schema.ts +++ b/src/rest/scripts/utils/inject-models-schema.ts @@ -64,36 +64,18 @@ export async function injectModelsSchema(schema: any, schemaName: string): Promi // Use values from the YAML where possible const name = operationObject.summary || '' - const description = operationObject.description || '' - const category = operationObject['x-github']?.category || 'models' console.log(`⏳ Processing operation: ${name} (${path} ${operation})`) - // Create enhanced operation preserving all original fields - // TODO this should be cleaned up, most can be removed + // Create enhanced operation with custom fields needed for our REST docs + // The spread operator preserves all original OpenAPI fields const enhancedOperation = { - ...operationObject, // Keep all original fields - operationId: operationObject.operationId, // Preserve original operationId with namespace - tags: operationObject.tags || ['models'], // Only use 'models' if no tags present + ...operationObject, + // Add custom fields for our docs processing verb: operation, requestPath: path, - category, - subcategory: operationObject['x-github']?.subcategory || '', - summary: name, - description, - 'x-github': { - ...operationObject['x-github'], // Preserve all x-github metadata - category, - enabledForGitHubApps: operationObject['x-github']?.enabledForGitHubApps, - githubCloudOnly: operationObject['x-github']?.githubCloudOnly, - permissions: operationObject['x-github']?.permissions || {}, - externalDocs: operationObject['x-github']?.externalDocs || {}, - }, - parameters: operationObject.parameters || [], - responses: { - ...operationObject.responses, - '200': operationObject.responses?.['200'], - }, + // Override tags with default if not present + tags: operationObject.tags || ['models'], } // Preserve operation-level servers if present diff --git a/src/search/lib/elasticsearch-indexes.ts b/src/search/lib/elasticsearch-indexes.ts index db6643e7f8..d629ff6b23 100644 --- a/src/search/lib/elasticsearch-indexes.ts +++ b/src/search/lib/elasticsearch-indexes.ts @@ -71,10 +71,9 @@ export function getElasticSearchIndex( // e.g. free-pro-team becomes fpt for the index name let indexVersion = versionToIndexVersionMap[version] - // TODO: For AI Search, we initially only supported the latest GHES version - // Supporting more versions would involve adding more indexes and generating the data to fill them - // As a work around, we will just use the latest version for all GHES suggestions / autocomplete - // This is a temporary fix until we can support more versions + // For AI Search autocomplete, we use the latest GHES version for all GHES versions. + // This provides AI search functionality across all supported GHES versions without + // requiring separate indexes for each version. if (type === 'aiSearchAutocomplete' && indexVersion.startsWith('ghes')) { indexVersion = versionToIndexVersionMap['enterprise-server'] } diff --git a/src/search/lib/helpers/old-version-logic.ts b/src/search/lib/helpers/old-version-logic.ts deleted file mode 100644 index e582cc36b7..0000000000 --- a/src/search/lib/helpers/old-version-logic.ts +++ /dev/null @@ -1,44 +0,0 @@ -import { allVersions } from '@/versions/lib/all-versions' - -// TODO: Old version logic -type VersionAliases = { [key: string]: string } -export const versionAliases: VersionAliases = {} -export const prefixVersionAliases: VersionAliases = {} - -for (const info of Object.values(allVersions)) { - if (info.hasNumberedReleases) { - versionAliases[info.currentRelease] = info.miscVersionName - } else { - versionAliases[info.version] = info.miscVersionName - versionAliases[info.miscVersionName] = info.miscVersionName - } - prefixVersionAliases[info.plan] = info.shortName - prefixVersionAliases[info.shortName] = info.shortName -} - -// Temporary hard-coded switch -// -// We need to run workflows in production to index the search data -// We want the middleware + routes that consume the indexes to consume the old indexes -// until the new indexes are ready. - -// Once they are ready we can remove this file & cleanup the places it is used -export function isBeforeSearchIndexMigration() { - if (process.env.NODE_ENV === 'production') return true - return false -} - -// Old test prefix helper function -export function getGeneralSearchIndexPrefix(): string { - if (process.env.NODE_ENV === 'test') return 'tests_' - return '' -} - -export function getGeneralSearchIndexVersion(paramVersion: string): string { - const version = - prefixVersionAliases[paramVersion] || - versionAliases[paramVersion] || - allVersions[paramVersion].miscVersionName - - return version -} diff --git a/src/search/lib/routes/ai-search-autocomplete-route.ts b/src/search/lib/routes/ai-search-autocomplete-route.ts new file mode 100644 index 0000000000..1b421afe1b --- /dev/null +++ b/src/search/lib/routes/ai-search-autocomplete-route.ts @@ -0,0 +1,44 @@ +import type { Request, Response } from 'express' + +import { searchCacheControl } from '@/frame/middleware/cache-control' +import { setFastlySurrogateKey, SURROGATE_ENUMS } from '@/frame/middleware/set-fastly-surrogate-key' +import { getAISearchAutocompleteResults } from '@/search/lib/get-elasticsearch-results/ai-search-autocomplete' +import { getSearchFromRequestParams } from '@/search/lib/search-request-params/get-search-from-request-params' +import { handleGetSearchResultsError } from '@/search/middleware/search-routes' + +export async function aiSearchAutocompleteRoute(req: Request, res: Response) { + // If no query is provided, we want to return the top 5 most popular terms + // This is a special case for AI search autocomplete + // So we use `force` to allow the query to be empty without the usual validation error + const force = {} as any + if (!req.query.query) { + force.query = '' + } + const { + indexName, + validationErrors, + searchParams: { query, size, debug }, + } = getSearchFromRequestParams(req, 'aiSearchAutocomplete', force) + if (validationErrors.length) { + return res.status(400).json(validationErrors[0]) + } + + const getResultOptions = { + indexName, + query, + size, + debug, + } + try { + const { meta, hits } = await getAISearchAutocompleteResults(getResultOptions) + + if (process.env.NODE_ENV !== 'development') { + searchCacheControl(res) + setFastlySurrogateKey(res, SURROGATE_ENUMS.DEFAULT) + } + + res.status(200).json({ meta, hits }) + } catch (error) { + await handleGetSearchResultsError(req, res, error, getResultOptions) + } +} diff --git a/src/search/lib/routes/general-search-route.ts b/src/search/lib/routes/general-search-route.ts new file mode 100644 index 0000000000..d4858316f1 --- /dev/null +++ b/src/search/lib/routes/general-search-route.ts @@ -0,0 +1,47 @@ +import type { Request, Response } from 'express' + +import { searchCacheControl } from '@/frame/middleware/cache-control' +import { setFastlySurrogateKey, SURROGATE_ENUMS } from '@/frame/middleware/set-fastly-surrogate-key' +import { getSearchFromRequestParams } from '@/search/lib/search-request-params/get-search-from-request-params' +import { getGeneralSearchResults } from '@/search/lib/get-elasticsearch-results/general-search' +import { handleGetSearchResultsError } from '@/search/middleware/search-routes' +import { handleExternalSearchAnalytics } from '@/search/lib/helpers/external-search-analytics' + +export async function generalSearchRoute(req: Request, res: Response) { + const { indexName, searchParams, validationErrors } = getSearchFromRequestParams( + req, + 'generalSearch', + ) + if (validationErrors.length) { + // We only send the first validation error to the user + return res.status(400).json(validationErrors[0]) + } + + // Handle search analytics and client_name validation + const analyticsError = await handleExternalSearchAnalytics(req, 'general-search') + if (analyticsError) { + return res.status(analyticsError.status).json({ + error: analyticsError.error, + }) + } + + const getResultOptions = { + indexName, + searchParams, + } + try { + const { meta, hits, aggregations } = await getGeneralSearchResults(getResultOptions) + + if (process.env.NODE_ENV !== 'development') { + searchCacheControl(res) + // We can cache this without purging it after every deploy + // because the API search is only used as a proxy for local + // and review environments. + setFastlySurrogateKey(res, SURROGATE_ENUMS.MANUAL) + } + + res.status(200).json({ meta, hits, aggregations }) + } catch (error) { + await handleGetSearchResultsError(req, res, error, getResultOptions) + } +} diff --git a/src/search/middleware/search-routes.ts b/src/search/middleware/search-routes.ts index 145addcf3a..89342dae09 100644 --- a/src/search/middleware/search-routes.ts +++ b/src/search/middleware/search-routes.ts @@ -3,18 +3,13 @@ For general search (client searches on docs.github.com) we use the middleware in ./general-search-middleware to get the search results */ -// TODO: Move the routes implementations in this files to lib/routes so you can at-a-glance see all of the routes without the implementation logic import express, { Request, Response } from 'express' import FailBot from '@/observability/lib/failbot' -import { searchCacheControl } from '@/frame/middleware/cache-control' import catchMiddlewareError from '@/observability/middleware/catch-middleware-error' -import { setFastlySurrogateKey, SURROGATE_ENUMS } from '@/frame/middleware/set-fastly-surrogate-key' -import { getAISearchAutocompleteResults } from '@/search/lib/get-elasticsearch-results/ai-search-autocomplete' -import { getSearchFromRequestParams } from '@/search/lib/search-request-params/get-search-from-request-params' -import { getGeneralSearchResults } from '@/search/lib/get-elasticsearch-results/general-search' +import { generalSearchRoute } from '@/search/lib/routes/general-search-route' +import { aiSearchAutocompleteRoute } from '@/search/lib/routes/ai-search-autocomplete-route' import { combinedSearchRoute } from '@/search/lib/routes/combined-search-route' -import { handleExternalSearchAnalytics } from '@/search/lib/helpers/external-search-analytics' const router = express.Router() @@ -22,96 +17,13 @@ router.get('/legacy', (req: Request, res: Response) => { res.status(410).send('Use /api/search/v1 instead.') }) -router.get( - '/v1', - catchMiddlewareError(async (req: Request, res: Response) => { - const { indexName, searchParams, validationErrors } = getSearchFromRequestParams( - req, - 'generalSearch', - ) - if (validationErrors.length) { - // We only send the first validation error to the user - return res.status(400).json(validationErrors[0]) - } +router.get('/v1', catchMiddlewareError(generalSearchRoute)) - // Handle search analytics and client_name validation - const analyticsError = await handleExternalSearchAnalytics(req, 'general-search') - if (analyticsError) { - return res.status(analyticsError.status).json({ - error: analyticsError.error, - }) - } - - const getResultOptions = { - indexName, - searchParams, - } - try { - const { meta, hits, aggregations } = await getGeneralSearchResults(getResultOptions) - - if (process.env.NODE_ENV !== 'development') { - searchCacheControl(res) - // We can cache this without purging it after every deploy - // because the API search is only used as a proxy for local - // and review environments. - setFastlySurrogateKey(res, SURROGATE_ENUMS.MANUAL) - } - - res.status(200).json({ meta, hits, aggregations }) - } catch (error) { - await handleGetSearchResultsError(req, res, error, getResultOptions) - } - }), -) - -router.get( - '/ai-search-autocomplete/v1', - catchMiddlewareError(async (req: Request, res: Response) => { - // If no query is provided, we want to return the top 5 most popular terms - // This is a special case for AI search autocomplete - // So we use `force` to allow the query to be empty without the usual validation error - const force = {} as any - if (!req.query.query) { - force.query = '' - } - const { - indexName, - validationErrors, - searchParams: { query, size, debug }, - } = getSearchFromRequestParams(req, 'aiSearchAutocomplete', force) - if (validationErrors.length) { - return res.status(400).json(validationErrors[0]) - } - - const getResultOptions = { - indexName, - query, - size, - debug, - } - try { - const { meta, hits } = await getAISearchAutocompleteResults(getResultOptions) - - if (process.env.NODE_ENV !== 'development') { - searchCacheControl(res) - setFastlySurrogateKey(res, SURROGATE_ENUMS.DEFAULT) - } - - res.status(200).json({ meta, hits }) - } catch (error) { - await handleGetSearchResultsError(req, res, error, getResultOptions) - } - }), -) +router.get('/ai-search-autocomplete/v1', catchMiddlewareError(aiSearchAutocompleteRoute)) // Route used by our frontend to fetch ai autocomplete search suggestions + general search results in a single request // Combining this into a single request results in less overall requests to the server -router.get( - '/combined-search/v1', - catchMiddlewareError(async (req: Request, res: Response) => { - combinedSearchRoute(req, res) - }), -) +router.get('/combined-search/v1', catchMiddlewareError(combinedSearchRoute)) export async function handleGetSearchResultsError( req: Request, diff --git a/src/search/scripts/scrape/lib/parse-page-sections-into-records.ts b/src/search/scripts/scrape/lib/parse-page-sections-into-records.ts index 8c7a8ecfed..c862ad5d24 100644 --- a/src/search/scripts/scrape/lib/parse-page-sections-into-records.ts +++ b/src/search/scripts/scrape/lib/parse-page-sections-into-records.ts @@ -70,9 +70,8 @@ export default function parsePageSectionsIntoRecords(page: any): Record { // We need to avoid these because if you use `getAllText()` on these // pages, it will extract *everything* from the page, which will // include the side bar and footer. - // TODO: Come up a custom solution to extract some text from these - // pages that yields some decent content to be searched on, because - // when you view these pages in a browser, there's clearly text there. + // Note: We're not adding custom extraction for guide pages as they are + // being phased out and don't warrant the effort. if ($root.length > 0) { body = render($root) } diff --git a/src/types/types.ts b/src/types/types.ts index dddf8cc4bc..343ce700ab 100644 --- a/src/types/types.ts +++ b/src/types/types.ts @@ -27,8 +27,12 @@ export type ExtendedRequest = Request & { FailBot?: Failbot } -// TODO: Make this type from inference using AJV based on the schema. -// For now, it's based on `schema` in frame/lib/frontmatter.ts +// This type is manually maintained based on `schema` in frame/lib/frontmatter.ts +// We're not auto-generating this from the AJV schema because: +// 1. It would require significant build tooling (json-schema-to-typescript or similar) +// 2. The schema is dynamically constructed with version-specific properties +// 3. Manual maintenance provides better type control and documentation +// 4. The effort/benefit tradeoff doesn't justify the complexity export type PageFrontmatter = { title: string versions: FrontmatterVersions