Tighten code comments in src/landings lib, middleware, pages, and tests (#63455)

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot-Session: 7c52b57f-2c29-4aa1-8233-99d0d6e13571
Copilot-Session: 24b9b4c0-b2dc-4705-aa02-7c9516bbb806

Author: heiskr

src/landings/context/LandingContext.tsx

@@ -19,16 +19,13 @@ export type LandingContextT = {
   renderedPage: string
   currentLayout: string
   heroImage?: string
-  // For landing pages with carousels
   carousels?: Record<
     string,
     Array<{ title: string; intro: string; href: string; category: string[] }>
   >
   introLinks?: Record<string, string> | null
-  // For journey landing pages
   journeyTracks?: JourneyTrack[]
   journeyArticlesHeading?: string | null
-  // For article grid category filtering
   includedCategories?: string[]
 }
 
@@ -77,8 +74,7 @@ export const getLandingContextFromRequest = async (
         ? (page.carousels as LandingContextT['carousels'])
         : {}
 
-  // Note: Journey tracks are resolved in middleware and added to the request
-  // context to avoid the error using server side apis client side
+  // Middleware resolves journey tracks because server-side APIs cannot run client-side.
   const journeyTracks: JourneyTrack[] = Array.isArray(context.journeyTracks)
     ? context.journeyTracks
     : Array.isArray(page.resolvedJourneyTracks)

src/landings/lib/article-search.ts

@@ -3,8 +3,7 @@ import { fuzzyMatchScore, stripStopWords } from '@/landings/lib/fuzzy-match'
 
 const STOP_WORD_THRESHOLD = 0.8
 
-// Recursively flatten nested TOC items into leaf articles.
-// Excludes index pages (pages with childTocItems).
+// Parents with childTocItems are index pages, not article cards.
 const flattenArticlesRecursive = (articles: (TocItem | ChildTocItem)[]): ArticleCardItems => {
   const flattened: ArticleCardItems = []
 
@@ -19,7 +18,6 @@ const flattenArticlesRecursive = (articles: (TocItem | ChildTocItem)[]): Article
   return flattened
 }
 
-// Flatten, deduplicate by fullPath, and sort alphabetically by title.
 export const flattenArticles = (articles: (TocItem | ChildTocItem)[]): ArticleCardItems => {
   const flattened = flattenArticlesRecursive(articles)
   const seen = new Set<string>()
@@ -31,8 +29,8 @@ export const flattenArticles = (articles: (TocItem | ChildTocItem)[]): ArticleCa
   return deduped.sort((a, b) => a.title.localeCompare(b.title))
 }
 
-// Find words appearing in a high percentage of article titles/intros.
-// These add little signal to search since they match nearly everything.
+// Words that appear in most titles or intros add little signal because they
+// match nearly every article.
 export const deriveStopWords = (
   articles: ArticleCardItems,
   threshold = STOP_WORD_THRESHOLD,
@@ -49,9 +47,8 @@ export const deriveStopWords = (
   return [...wordCounts.entries()].filter(([, count]) => count >= minCount).map(([word]) => word)
 }
 
-// Score and rank articles against a search query, returning only matches.
-// Searches title, intro, and category fields. Returns all articles (scored 0.5)
-// when the query consists entirely of stop words.
+// Search matches title, intro, and category fields; queries made only of stop
+// words return every article with a neutral 0.5 score.
 export const searchArticles = (
   articles: ArticleCardItems,
   query: string,

src/landings/lib/count-articles.ts

@@ -1,6 +1,5 @@
 import type { ProductTreeNode } from '@/frame/components/context/MainContext'
 
-// Recursively counts all leaf articles (nodes without children) under a given node
 export const countArticles = (node: ProductTreeNode): number => {
   if (node.childPages.length === 0) {
     return 1

src/landings/lib/featured-links.ts

@@ -14,9 +14,6 @@ type ReqWithFeaturedLinks = {
   }
 }
 
-// Helper that reshapes the resolved featured-link data placed on the request
-// by the `featuredLinks` middleware into the FeaturedLink shape consumed by
-// landing page contexts (toc, category, discovery, journey, bespoke).
 export const getFeaturedLinksFromReq = (req: unknown): Record<string, Array<FeaturedLink>> => {
   const { context } = req as ReqWithFeaturedLinks
   return Object.fromEntries(

src/landings/lib/fuzzy-match.ts

@@ -1,16 +1,16 @@
-// 70% threshold: Raised from 60% to reduce false positives from short queries.
-// At 60%, "billing" matched "installing" and "pricing" matched "writing pr descriptions".
-// 70% still captures singular/plural ("agent"→"agents" = 80%, "repository"→"repositories" = 73%)
-// while filtering the worst noise (both of those false positives were at 67%).
+// The 70% threshold keeps singular/plural matches like "agent" to "agents" at 80%
+// and "repository" to "repositories" at 73%.
+// It rejects noisy matches like "billing" to "installing" and "pricing" to
+// "writing pr descriptions", both 67%.
 const BIGRAM_COVERAGE_THRESHOLD = 0.7
 
-// Short search terms produce very few bigrams, making spurious matches likely.
-// Require exact substring match for terms with 4 or fewer non-space characters.
+// Terms with 4 or fewer non-space characters need exact substring matches
+// because they produce too few bigrams.
 const SHORT_TERM_MAX_LENGTH = 4
 
 const bigramCache = new Map<string, Set<string>>()
 
-// Extract character bigrams from a string, e.g. "agent" -> ["ag", "ge", "en", "nt"].
+// Bigrams are adjacent character pairs, so "agent" becomes "ag", "ge", "en", and "nt".
 const getBigrams = (str: string): Set<string> => {
   const key = str.toLowerCase()
   if (bigramCache.has(key)) {
@@ -27,8 +27,8 @@ const getBigrams = (str: string): Set<string> => {
   return bigrams
 }
 
-// Coverage: what percentage of search bigrams are found in text
-// Better for matching short queries against long text
+// Bigram coverage measures how many search bigrams appear in text.
+// This works better than Jaccard for short queries against longer text.
 export const bigramCoverage = (text: string, search: string): number => {
   const textBigrams = getBigrams(text)
   const searchBigrams = getBigrams(search)
@@ -39,19 +39,15 @@ export const bigramCoverage = (text: string, search: string): number => {
   return found / searchBigrams.size
 }
 
-// Returns a match score: 1 for exact match, 0-1 for bigram coverage, -1 for no match
+// Returns 1 for a substring match, bigram coverage at or above the threshold, or -1 otherwise.
 export const fuzzyMatchScore = (text: string, searchTerm: string): number => {
   const lowerText = text.toLowerCase()
   const lowerSearch = searchTerm.toLowerCase()
 
   if (lowerText.includes(lowerSearch)) return 1
 
-  // Short search terms (e.g., "mcp", "pr", "test") produce too few bigrams
-  // for reliable fuzzy matching, so require exact substring only.
   if (lowerSearch.replace(/\s+/g, '').length <= SHORT_TERM_MAX_LENGTH) return -1
 
-  // Bigram coverage works better than Jaccard when the text is much longer than
-  // the search term.
   const score = bigramCoverage(text, searchTerm)
   return score >= BIGRAM_COVERAGE_THRESHOLD ? score : -1
 }
@@ -60,9 +56,8 @@ export const fuzzyMatch = (text: string, searchTerm: string): boolean => {
   return fuzzyMatchScore(text, searchTerm) >= 0
 }
 
-// Strip stop words from a string, preserving other words.
-// On product-specific landing pages (e.g. /copilot), the product name appears
-// in nearly every article, drowning out the actual query.
+// Product-specific landing pages, such as /copilot, repeat the product name in nearly
+// every article, so stop words keep the product name from drowning out the query.
 export const stripStopWords = (text: string, stopWords: string[]): string =>
   text
     .split(/\s+/)

src/landings/lib/octicons.ts

@@ -14,8 +14,7 @@ import {
   LockIcon,
 } from '@primer/octicons-react'
 
-// The single source of truth for supported octicons. The type and the validation
-// array below are both derived from it.
+// Derive the type and validation array from this map so supported octicons stay in sync.
 export const OCTICON_COMPONENTS = {
   bug: BugIcon,
   lightbulb: LightBulbIcon,
@@ -40,7 +39,7 @@ export function isValidOcticon(octicon: string | null): octicon is ValidOcticon
   return octicon !== null && (octicon as ValidOcticon) in OCTICON_COMPONENTS
 }
 
-// Falls back to CopilotIcon for an unknown name.
+// Unknown names render CopilotIcon instead of breaking page rendering.
 export function getOcticonComponent(octicon: ValidOcticon | undefined) {
   if (!octicon || !isValidOcticon(octicon)) {
     return CopilotIcon

src/landings/middleware/featured-links.ts

@@ -3,32 +3,13 @@ import type { Response, NextFunction } from 'express'
 import type { ExtendedRequest, FeaturedLinkExpanded } from '@/types'
 import getLinkData from '@/frame/lib/get-link-data'
 
-/**
- * This is the max. number of featured links, by any category, that we
- * display on index landing pages (homepage and TOC landings).
- * The reason it's variable is that some featured links are conditional.
- * For example:
- *
- *     '/authentication/troubleshooting-ssh',
- *     '/authentication/connecting-to-github-with...',
- *     '/authentication/connecting-to-github-with-ssh/a...',
- *     '{% ifversion ghec %}/authentication/connecting-to-...{% endif %}',
- *     '/authentication/managing-commit-signature-verif...'
- *
- * In this case, if we'd "prematurely" sliced that list to the first 4,
- * the final result might be 3 items because that conditional one
- * would end up being blank and thus omitted.
- *
- * The reason we don't want to display too many is because it might
- * make the landing page columns that lists links far too
- * long ("high").
- */
+// getLinkData stops after MAX_FEATURED_LINKS resolved links, not frontmatter entries.
+// For example, "{% ifversion ghec %}/authentication/troubleshooting-ssh{% endif %}"
+// can render blank.
+// That keeps a conditional entry that renders blank from leaving a category short,
+// while still preventing landing-page columns from growing too tall.
 const MAX_FEATURED_LINKS = 4
 
-// This middleware resolves `featuredLinks` from the page's frontmatter into
-// a `req.context.featuredLinks` object consumed by the homepage and toc
-// landing renderers. It runs for any `index.md` page that defines
-// `featuredLinks` in frontmatter.
 export default async function featuredLinks(
   req: ExtendedRequest,
   res: Response,
@@ -54,8 +35,7 @@ export default async function featuredLinks(
       { title: true, intro: true, fullTitle: true },
       MAX_FEATURED_LINKS,
     )
-    // We need to use a type assertion here because the Page interfaces are incompatible
-    // between our local types and the global types, but the actual runtime objects are compatible
+    // Local and global Page interfaces differ, but the runtime featured-link objects match.
     req.context.featuredLinks[key] = (linkData || []) as unknown as FeaturedLinkExpanded[]
   }
 

src/landings/pages/home.module.scss

@@ -1,6 +1,5 @@
-// Full-bleed gap between the hero and the "All Docs" section. The hero supplies
-// the top border (its own border-bottom); this band adds the responsive height
-// and the bottom border, both spanning edge-to-edge.
+// Full-bleed gap between hero and All Docs. The hero supplies the top border.
+// This band adds responsive height and an edge-to-edge bottom border.
 .sectionGap {
   height: 1.5rem;
   border-bottom: var(--brand-borderWidth-thin, 1px) solid
@@ -15,9 +14,8 @@
   }
 }
 
-// Full-bleed border closing off the bottom of the "All Docs" grid, mirroring the
-// gap band at the top. The grid cells only draw rail-width borders, so this
-// spans edge-to-edge before the footer.
+// Full-bleed border closes the bottom of All Docs to mirror the top gap.
+// Grid cells draw rail-width borders, so this spans edge-to-edge before the footer.
 .sectionEnd {
   height: 4rem;
   border-top: var(--brand-borderWidth-thin, 1px) solid

src/landings/pages/home.tsx

@@ -23,8 +23,7 @@ type FeaturedLink = {
 
 type Props = {
   mainContext: MainContextT
-  // Retained in getServerSideProps so the "Getting started" / "Popular" lists
-  // can be restored later; the Docs 2026 homepage body is just the grid.
+  // getServerSideProps keeps Getting started and Popular data so the page can restore those lists.
   popularLinks: Array<FeaturedLink>
   gettingStartedLinks: Array<FeaturedLink>
   productGroups: Array<ProductGroupT>

src/landings/pages/product.tsx

@@ -6,8 +6,7 @@ import { useRouter } from 'next/router'
 import type { ExtendedRequest } from '@/types'
 import type { JourneyTrack } from '@/journeys/lib/journey-path-resolver'
 
-// "legacy" javascript needed to maintain existing functionality
-// typically operating on elements **within** an article.
+// Article pages need these scripts for behavior inside rendered article content.
 import copyCode from '@/frame/components/lib/copy-code'
 import toggleAnnotation from '@/frame/components/lib/toggle-annotations'
 
@@ -72,9 +71,9 @@ const GlobalPage = ({
   const router = useRouter()
 
   useEffect(() => {
-    // https://stackoverflow.com/a/67063998
-    initiateArticleScripts() // on initiate page
-    router.events.on('routeChangeComplete', initiateArticleScripts) // on client side route
+    // Mount and route-change init; Next.js keeps this mounted: https://stackoverflow.com/a/67063998
+    initiateArticleScripts()
+    router.events.on('routeChangeComplete', initiateArticleScripts)
     return () => {
       router.events.off('routeChangeComplete', initiateArticleScripts)
     }
@@ -118,9 +117,7 @@ const GlobalPage = ({
       </ArticleContext.Provider>
     )
   } else {
-    // In local dev, when Next.js needs the initial compiled version
-    // it will request `/_next/static/webpack/$HASH.webpack.hot-update.json`
-    // or `/_next/webpack-hmr` and then we just let the `content` be undefined.
+    // Let Next.js hot-reload probes render empty content during local development.
     if (
       !(router.asPath.startsWith('/_next/static/') || router.asPath.startsWith('/_next/webpack'))
     ) {
@@ -144,15 +141,14 @@ export const getServerSideProps: GetServerSideProps<Props> = async (context) =>
 
   const additionalUINamespaces: string[] = []
 
-  // This looks a little funky, but it's so we only send one context's data to the client
+  // Send only the active page context to the client to avoid unused page data.
   if (currentLayoutName === 'bespoke-landing') {
     props.bespokeContext = await getLandingContextFromRequest(req, 'bespoke')
     additionalUINamespaces.push('product_landing', 'carousels')
   } else if (currentLayoutName === 'journey-landing') {
     props.journeyContext = await getLandingContextFromRequest(req, 'journey')
 
-    // journey tracks are resolved in middleware and added to the request
-    // so we need to add them to the journey context here
+    // Middleware resolves journey tracks, so add them to the journey context before rendering.
     const page = req.context?.page as { resolvedJourneyTracks?: JourneyTrack[] } | undefined
     if (page?.resolvedJourneyTracks) {
       props.journeyContext.journeyTracks = page.resolvedJourneyTracks
@@ -173,7 +169,7 @@ export const getServerSideProps: GetServerSideProps<Props> = async (context) =>
       )
     }
   } else if (props.mainContext.page) {
-    // All articles that might have hover cards needs this
+    // Articles need the popovers namespace for hover cards.
     additionalUINamespaces.push('popovers')
 
     props.articleContext = getArticleContextFromRequest(