Tighten code comments in src/graphql components, lib, pages, and tests (#63460)

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

Author: heiskr

src/graphql/components/GraphqlCategoryPage.module.scss

@@ -1,22 +1,16 @@
-// Heading rhythm tweaks specific to GraphQL category pages.
-// Each schema kind (Objects, Mutations, etc.) is a section H2; individual
-// items inside a section are H3; sub-sections within an item (fields,
-// arguments, return fields, etc.) are H4.
+// GraphQL category pages render kinds as H2, items as H3, and item subsections as H4.
 .categoryPage {
   h2 {
     padding-top: 2.5rem;
   }
 
-  // Items inside a kind section need top spacing so consecutive items don't
-  // visually run together. The first item under each kind H2 doesn't need
-  // it: the H2's own padding-top already provides plenty of breathing room.
+  // Items inside a kind section need top spacing so consecutive items do not run together.
+  // The first item under each kind H2 relies on the H2 padding instead.
   h3 {
     padding-top: 2rem;
   }
 
-  // The very first H2 on the page sits directly below the intro/lead; the
-  // extra padding-top adds an awkward gap there. Same idea for the first H3
-  // in each section, which sits directly under its kind H2.
+  // Skip top padding where the heading follows the intro or its kind H2, to avoid a double gap.
   > :first-child h2:first-child,
   section > h3:first-of-type {
     padding-top: 0;
@@ -26,15 +20,8 @@
     padding-top: 0;
   }
 
-  // The schema item description is rendered as HTML and often wraps in a
-  // single <p>. The trailing paragraph margin leaves a "chin" between the
-  // description and the next sub-section, so strip it. The
-  // `graphql-item-description` class is added to the description wrapper
-  // inside `GraphqlItem` by sibling PR #61435; until that merges, this rule
-  // is inert (and it's only ever evaluated under `.categoryPage`, which
-  // doesn't render until the recat PR wires this component up).
-  // `:global` is required because CSS modules would otherwise rewrite the
-  // class name.
+  // GraphQL descriptions often render as one p whose bottom margin leaves a gap.
+  // CSS modules would rewrite graphql-item-description unless :global keeps the emitted class.
   :global(.graphql-item-description) > :last-child {
     margin-bottom: 0;
   }

src/graphql/components/GraphqlCategoryPage.tsx

@@ -49,27 +49,16 @@ export type CategorySchema = Partial<{
 
 type Props = {
   schema: CategorySchema
-  // All objects across every category. Used by `Interface` to list
-  // implementers regardless of which category page is being rendered.
+  // Interface needs objects from every category to list implementers across category pages.
   allObjects: ObjectT[]
 }
 
-// Item-level heading level used when items render under a kind section
-// heading (`<h2>`). Kept in one place so the matching mini-TOC builder in
-// `pages/reference.tsx` can stay in sync with the on-page anchors.
+// Keep item headings in sync with the mini-TOC builder in src/graphql/pages/reference.tsx.
 const ITEM_HEADING_LEVEL = 3
 
+// src/graphql/pages/reference.tsx sends empty categories to 404, so this renders populated pages.
+// GraphqlItem keeps kind labels so deep-linked items remain self-describing outside the section.
 export function GraphqlCategoryPage({ schema, allObjects }: Props) {
-  // Render one section per kind, in the canonical `ALL_KIND_KEYS` order.
-  // Items inside each section are sorted case-insensitively by name. The
-  // per-kind label pill rendered by `GraphqlItem` is now somewhat redundant
-  // here (the kind is obvious from the section heading directly above), but
-  // we keep it for now so items stay visually self-describing if they're
-  // ever deep-linked or rendered outside the section context.
-  //
-  // Empty-category pages are short-circuited to a 404 in
-  // `pages/reference.tsx`, so this component is only ever rendered with at
-  // least one section.
   const sections = ALL_KIND_KEYS.flatMap((kind) => {
     const items = schema[kind]
     if (!items || items.length === 0) return []
@@ -123,6 +112,4 @@ function renderItem(kind: SchemaKindKey, item: AnySchemaItem, allObjects: Object
   }
 }
 
-// Re-export the kind label map for callers that want to render a label
-// outside of the page (e.g. mini-toc or breadcrumbs).
 export { KIND_LABELS }

src/graphql/components/GraphqlItem.tsx

@@ -12,15 +12,12 @@ type Props = {
   heading?: string
   headingLevel?: number
   children?: React.ReactNode
-  // When provided, the heading id is prefixed with the kind so two items
-  // with the same case-insensitive name across kinds get distinct anchors
-  // on a category page (e.g. `object-repository` vs `query-repository`).
+  // Prefix heading IDs so names shared across kinds get distinct anchors.
+  // For example, object-repository and query-repository can coexist.
   kind?: SchemaKindKey
 }
 
-// Clamp a numeric heading level to the valid HTML range (2-6). Used to
-// build heading tag names like `h2`/`h3` from a numeric `headingLevel`
-// prop without producing invalid tags if a caller passes something odd.
+// Clamp heading tags to h2 through h6 when callers pass odd headingLevel values.
 function headingTag(level: number): keyof JSX.IntrinsicElements {
   const clamped = Math.max(2, Math.min(6, level))
   return `h${clamped}` as keyof JSX.IntrinsicElements
@@ -31,9 +28,7 @@ export function GraphqlItem({ item, heading, children, headingLevel = 2, kind }:
   const slug = kind ? `${KIND_SLUG_PREFIX[kind]}-${baseSlug}` : baseSlug
   const hasNotice = Boolean(item.preview || item.isDeprecated)
   const kindLabel = kind ? KIND_LABELS[kind] : undefined
-  // Sub-headings rendered via the `heading` prop should sit one level below
-  // the item's own heading so the document outline stays well-formed when
-  // the item itself is nested under a kind section heading on category pages.
+  // Subheadings sit one level below the item heading to keep category page outlines valid.
   const SubHeading = headingTag(headingLevel + 1)
 
   return (
@@ -61,6 +56,5 @@ export function GraphqlItem({ item, heading, children, headingLevel = 2, kind }:
   )
 }
 
-// Re-exported so per-kind wrappers can build matching sub-sub-headings
-// (e.g. Mutation's "Return fields" h-tag) without duplicating the clamp logic.
+// Per-kind wrappers share headingTag so Mutation return fields use the same heading clamp.
 export { headingTag }

src/graphql/components/GraphqlPage.tsx

@@ -28,10 +28,8 @@ type Props = {
 }
 
 export const GraphqlPage = ({ schema, pageName, objects }: Props) => {
-  const graphqlItems: JSX.Element[] = [] // In the case of the H2s for Queries
+  const graphqlItems: JSX.Element[] = []
 
-  // The queries page has two heading sections (connections and fields), so add
-  // the heading component and its children once per section.
   if (pageName === 'queries') {
     graphqlItems.push(
       ...(schema as QueryT[]).map((item) => <Query item={item} key={item.id + item.name} />),

src/graphql/lib/categories.ts

@@ -1,9 +1,4 @@
-// Canonical mapping of internal schema kinds to:
-// - urlKind: the URL/folder segment used in href anchors before categorization
-//   (kept for backward-compat with `helpers.getFullLink` signature)
-// - slugPrefix: the kind-disambiguating slug prefix used on category pages
-//   so two items sharing a case-insensitive name don't collide
-// - label: human-readable label rendered as a Primer Label next to each item
+// Schema kind tables keep legacy URL segments, category-page slug prefixes, and visible labels.
 
 export type SchemaKindKey =
   | 'queries'
@@ -26,8 +21,7 @@ export const KIND_LABELS: Record<SchemaKindKey, string> = {
   scalars: 'Scalar',
 }
 
-// Plural form of `KIND_LABELS`, used as the section heading (and mini-TOC
-// parent label) when a GraphQL category page groups its items by kind.
+// Plural labels appear in category-page sections and mini-TOC section entries.
 export const KIND_LABELS_PLURAL: Record<SchemaKindKey, string> = {
   queries: 'Queries',
   mutations: 'Mutations',
@@ -39,10 +33,7 @@ export const KIND_LABELS_PLURAL: Record<SchemaKindKey, string> = {
   scalars: 'Scalars',
 }
 
-// Slug prefix used to disambiguate items across kinds on a category page.
-// For example, a `Repository` object and a `repository` query both have id
-// `repository`; on a category page they become `object-repository` and
-// `query-repository` respectively.
+// Category-page anchors prefix the kind, so Repository object and repository query stay distinct.
 export const KIND_SLUG_PREFIX: Record<SchemaKindKey, string> = {
   queries: 'query',
   mutations: 'mutation',
@@ -54,9 +45,8 @@ export const KIND_SLUG_PREFIX: Record<SchemaKindKey, string> = {
   scalars: 'scalar',
 }
 
-// The "URL kind" / `pageType` value used by `helpers.getTypeKind` and
-// `helpers.getFullLink`. `inputObjects` (camelCase internal key) becomes
-// `input-objects` in URLs.
+// These URL segments match helpers.getTypeKind output and helpers.getFullLink input.
+// For example, inputObjects becomes input-objects.
 export const KIND_URL_SEGMENT: Record<SchemaKindKey, string> = {
   queries: 'queries',
   mutations: 'mutations',
@@ -79,28 +69,22 @@ export const ALL_KIND_KEYS: SchemaKindKey[] = [
   'scalars',
 ]
 
-// Reverse map from the URL-kind segment used in hrefs (e.g. `input-objects`)
-// to the slug prefix used to disambiguate items in category page anchors
-// (e.g. `input-object`). Derived from KIND_URL_SEGMENT + KIND_SLUG_PREFIX so
-// the three tables stay in sync automatically.
+// Derive URL-segment to slug-prefix mappings from the source tables so anchor helpers stay in sync.
+// For example, input-objects maps to input-object.
 export const SLUG_PREFIX_BY_URL_SEGMENT: Record<string, string> = Object.fromEntries(
   ALL_KIND_KEYS.map((k) => [KIND_URL_SEGMENT[k], KIND_SLUG_PREFIX[k]]),
 )
 
-// Given a URL-kind segment (as returned by helpers.getTypeKind, e.g.
-// `objects`, `input-objects`), return the slug prefix used to disambiguate
-// items in category page anchors. Falls back to the input for unknown kinds.
+// Unknown URL-kind segments fall back to themselves so callers can handle future kinds.
 export function slugPrefixForUrlKind(urlKind: string): string {
   return SLUG_PREFIX_BY_URL_SEGMENT[urlKind] ?? urlKind
 }
 
-// Bucket all items that don't have an upstream `@docsCategory` directive.
+// Unannotated upstream schema items fall into the other category.
 export const OTHER_CATEGORY = 'other'
 
-// Canonical list of categories emitted by the upstream `docs_category` DSL.
-// Keep this list in sync with the allowlist in
-// `github/github`'s `app/platform/objects/base/docs_category.rb`.
-// `other` is a docs-internal bucket for un-annotated types.
+// github/github app/platform/objects/base/docs_category.rb must allow each upstream category here.
+// The other category belongs to docs-internal for unannotated types.
 export const CATEGORIES = [
   'actions',
   'activity',
@@ -153,7 +137,6 @@ export function isValidCategory(slug: string): slug is CategorySlug {
   return (CATEGORIES as readonly string[]).includes(slug)
 }
 
-// Human-readable display title for a category. Falls back to slug.
 export function categoryTitle(slug: string): string {
   switch (slug) {
     case 'apps':

src/graphql/lib/index.ts

@@ -13,37 +13,31 @@ import type {
 } from '@/graphql/components/types'
 import { ALL_KIND_KEYS, CATEGORIES, isValidCategory, type SchemaKindKey } from './categories'
 
-// GraphqlContext describes the per-request context object that getMiniToc and
-// getGraphqlSchema read language/version from.
 export interface GraphqlContext {
   currentLanguage: string
   currentVersion: string
   [key: string]: unknown
 }
 
-// The GraphQL schema JSON is keyed by member type (e.g. "queries", "objects",
-// "enums"), each holding a list of schema members.
+// GraphQL schema JSON groups members by schema kind.
 type GraphqlSchemaData = Record<string, GraphqlT[]>
 
 export const GRAPHQL_DATA_DIR = 'src/graphql/data'
-/* ADD LANGUAGE KEY */
 const previews = new Map<string, PreviewT[]>()
 const upcomingChanges = new Map<string, BreakingChangesT>()
 const changelog = new Map<string, ChangelogItemT[]>()
 const changelogMiniTocs = new Map<string, MiniTocItem[]>()
-// Per-category schema files. Key: `${graphqlVersion}:${category}` → bucket.
+// Per-category schema cache keys combine graphqlVersion and category.
 const graphqlCategorySchemas = new Map<string, GraphqlSchemaData>()
-// All objects across categories (for interface implementer lookup).
+// Interface renderers need object items from every category to list implementers.
 const allObjectsByVersion = new Map<string, GraphqlT[]>()
 const miniTocs = new Map<string, Map<string, Map<string, MiniTocItem[]>>>()
 
 for (const language of Object.keys(languages)) {
   miniTocs.set(language, new Map())
 }
 
-// Returns the per-category schema bucket `{queries, mutations, ...}` for a
-// given category slug (e.g. 'repos', 'issues'). Throws via the loader if the
-// category slug is not valid for this version.
+// Reject invalid category slugs before the loader reads a missing schema file.
 export function getGraphqlSchema(version: string, category: string): GraphqlSchemaData {
   if (!isValidCategory(category)) {
     throw new Error(`Invalid GraphQL category: ${category}`)
@@ -65,9 +59,7 @@ function getGraphqlSchemaByCategory(graphqlVersion: string, category: string): G
   return graphqlCategorySchemas.get(key)!
 }
 
-// Returns all object-kind items across every category for the given version.
-// Used by the interface renderer to list implementers regardless of which
-// category page is being rendered.
+// Interface renderers need objects from every category to list implementers.
 export function getAllGraphqlObjects(version: string): GraphqlT[] {
   const graphqlVersion: string = getGraphqlVersion(version)
   if (!allObjectsByVersion.has(graphqlVersion)) {
@@ -81,7 +73,6 @@ export function getAllGraphqlObjects(version: string): GraphqlT[] {
   return allObjectsByVersion.get(graphqlVersion)!
 }
 
-// Returns the canonical render order of kinds within a category page.
 export function getKindOrder(): SchemaKindKey[] {
   return ALL_KIND_KEYS
 }
@@ -100,17 +91,11 @@ export function getGraphqlChangelog(version: string): ChangelogItemT[] {
   return changelog.get(graphqlVersion)!
 }
 
-/**
- * Return changelog entries filtered by year.
- */
 export function getGraphqlChangelogByYear(version: string, year: number): ChangelogItemT[] {
   const all = getGraphqlChangelog(version)
   return all.filter((entry) => entry.date.startsWith(String(year)))
 }
 
-/**
- * Return the distinct years present in the changelog, sorted descending (newest first).
- */
 export function getGraphqlChangelogYears(version: string): number[] {
   const all = getGraphqlChangelog(version)
   const years = new Set<number>()

src/graphql/lib/validator.ts

@@ -1,5 +1,4 @@
-// the tests in tests/graphql.ts use this schema to ensure the integrity
-// of the data in src/graphql/data/*.json
+// src/graphql/tests/validate-schema.ts reads these schemas to validate generated data.
 
 interface JSONSchema {
   type?: string
@@ -79,7 +78,6 @@ export const upcomingChangesValidator: ValidatorSchema = {
   },
 }
 
-// many GraphQL schema members have these core properties
 const coreProps: JSONSchema = {
   properties: {
     name: {
@@ -107,7 +105,6 @@ const coreProps: JSONSchema = {
   },
 }
 
-// some GraphQL schema members have the core properties plus an 'args' object
 const corePropsPlusArgs = dup(coreProps)
 
 corePropsPlusArgs.properties!.args = {
@@ -118,7 +115,6 @@ corePropsPlusArgs.properties!.args = {
   },
 }
 
-// the args object can have defaultValue prop
 corePropsPlusArgs.properties!.args.items!.properties!.defaultValue = {
   type: 'boolean',
 }

src/graphql/pages/breaking-changes.tsx

@@ -49,9 +49,7 @@ export const getServerSideProps: GetServerSideProps<Props> = async (context) =>
   const schema = getGraphqlBreakingChanges(currentVersion)
   if (!schema) throw new Error(`No graphql breaking changes schema found for ${currentVersion}`)
 
-  // Gets the miniTocItems in the article context. At this point it will only
-  // include miniTocItems that exist in Markdown pages in
-  // content/graphql/reference/*
+  // Start from the current page's Markdown headings, then append the generated ones.
   const automatedPageContext = getAutomatedPageContextFromRequest(req)
   const slugger = new GithubSlugger()
   const headings = Object.fromEntries(
@@ -69,7 +67,6 @@ export const getServerSideProps: GetServerSideProps<Props> = async (context) =>
   )
   const titles = Object.values(headings).map((heading) => heading.title)
   const changelogMiniTocItems = await getAutomatedPageMiniTocItems(titles, req.context!, 2)
-  // Update the existing context to include the miniTocItems from GraphQL
   automatedPageContext.miniTocItems.push(...changelogMiniTocItems)
 
   return {

src/graphql/pages/changelog.tsx

@@ -69,10 +69,7 @@ export const getServerSideProps: GetServerSideProps<Props> = async (context) =>
   }
 }
 
-/**
- * Strip wrapping `<p>` tags from HTML change descriptions to allow
- * rendering as `<li>` content without nested block elements.
- */
+// Strip wrapping p tags so list items do not contain nested block elements.
 export function stripParagraphWrappers(schema: ChangelogItemT[]) {
   for (const item of schema) {
     for (const group of [item.schemaChanges, item.previewChanges, item.upcomingChanges]) {