Improve description of responses with the same status code

.chronus/changes/improve-response-descriptions-same-status-code-2026-8-9-2-8-16.md

@@ -0,0 +1,20 @@
+---
+changeKind: fix
+packages:
+  - "@typespec/http"
+  - "@typespec/openapi3"
+---
+
+Improve how response descriptions are chosen when multiple responses share a status code. When an operation has multiple responses with the same HTTP status code, all responses sharing that code are considered when computing the description associated with that code instead of just one.
+
+```tsp
+@doc("A cat.")
+model Cat { @statusCode _: 200, meow: boolean }
+
+@doc("A dog.")
+model Dog { @statusCode _: 200, bark: boolean }
+
+// The response description defaults to "The request has succeeded."
+// because the doc comments for the variants conflict.
+op read(): Cat | Dog;
+```

packages/http/src/responses.ts

@@ -26,7 +26,13 @@ import type { HttpProperty } from "./http-property.js";
 import { HttpStateKeys, reportDiagnostic } from "./lib.js";
 import { Visibility } from "./metadata.js";
 import { HttpPayloadDisposition, resolveHttpPayload } from "./payload.js";
-import type { HttpOperationResponse, HttpStatusCodes, HttpStatusCodesEntry } from "./types.js";
+import type {
+  HttpOperationResponse,
+  HttpOperationResponseContent,
+  HttpPayloadBody,
+  HttpStatusCodes,
+  HttpStatusCodesEntry,
+} from "./types.js";
 
 /**
  * Get the responses for a given operation.
@@ -36,16 +42,49 @@ export function getResponsesForOperation(
   operation: Operation,
 ): [HttpOperationResponse[], readonly Diagnostic[]] {
   const diagnostics = createDiagnosticCollector();
-  const responses = new ResponseIndex();
 
   // Resolve union variants into concrete response types, grouping plain body variants
   // (no HTTP metadata) into a single union type.
   const variants = resolveResponseVariants(program, operation.returnType);
+  const processedResponses: ProcessedResponseType[] = [];
   for (const { type, description } of variants) {
-    processResponseType(program, diagnostics, operation, responses, type, description);
+    processedResponses.push(
+      processResponseType(program, diagnostics, operation, type, description),
+    );
+  }
+
+  const responsesByStatus = new ResponseIndex();
+  for (const response of processedResponses) {
+    for (const statusCode of response.statusCodes) {
+      responsesByStatus.add(statusCode, response);
+    }
+  }
+
+  const responses: HttpOperationResponse[] = [];
+  for (const [statusCode, responseGroup] of responsesByStatus.entries()) {
+    const responseContents: HttpOperationResponseContent[] = responseGroup.map((response) => {
+      const content: HttpOperationResponseContent = {
+        headers: response.headers,
+        properties: response.properties,
+      };
+      if (response.body) {
+        content.body = response.body;
+      }
+      return content;
+    });
+
+    responses.push({
+      statusCodes: statusCode,
+      // It would be more accurate to express the response type as a union of all variant types.
+      // However, downstream code written since we first decided to use only the first variant's
+      // type may rely on us to continue doing so. For now, keep this behavior.
+      type: responseGroup[0].type,
+      description: getResponsesDescription(program, operation, statusCode, responseGroup),
+      responses: responseContents,
+    });
   }
 
-  return diagnostics.wrap(responses.values());
+  return diagnostics.wrap(responses);
 }
 
 interface ResolvedResponseVariant {
@@ -110,37 +149,57 @@ function resolveResponseVariants(
  * Class keeping an index of all the response by status code
  */
 class ResponseIndex {
-  readonly #index = new Map<string, HttpOperationResponse>();
+  readonly #index = new Map<string, ProcessedResponseType[]>();
 
-  public get(statusCode: HttpStatusCodesEntry): HttpOperationResponse | undefined {
-    return this.#index.get(this.#indexKey(statusCode));
-  }
-
-  public set(statusCode: HttpStatusCodesEntry, response: HttpOperationResponse): void {
-    this.#index.set(this.#indexKey(statusCode), response);
+  public add(statusCode: HttpStatusCodesEntry, response: ProcessedResponseType): void {
+    const indexKey = this.#indexKey(statusCode);
+    if (this.#index.has(indexKey)) {
+      this.#index.get(indexKey)!.push(response);
+      return;
+    }
+    this.#index.set(indexKey, [response]);
   }
 
-  public values(): HttpOperationResponse[] {
-    return [...this.#index.values()];
+  public *entries(): MapIterator<[HttpStatusCodesEntry, ProcessedResponseType[]]> {
+    for (const [indexKey, responses] of this.#index.entries()) {
+      let parsedStatusCodes: HttpStatusCodesEntry;
+      if (indexKey === "*") {
+        parsedStatusCodes = "*";
+      } else if (indexKey.includes(",")) {
+        const [start, end] = indexKey.split(",");
+        parsedStatusCodes = { start: Number(start), end: Number(end) };
+      } else {
+        parsedStatusCodes = Number(indexKey);
+      }
+      yield [parsedStatusCodes, responses];
+    }
   }
 
   #indexKey(statusCode: HttpStatusCodesEntry) {
     if (typeof statusCode === "number" || statusCode === "*") {
       return String(statusCode);
     } else {
-      return `${statusCode.start}-${statusCode.end}`;
+      return `${statusCode.start},${statusCode.end}`;
     }
   }
 }
 
+interface ProcessedResponseType {
+  statusCodes: HttpStatusCodes;
+  type: Type;
+  parentDescription?: string;
+  body?: HttpPayloadBody;
+  headers: Record<string, ModelProperty>;
+  properties: HttpProperty[];
+}
+
 function processResponseType(
   program: Program,
   diagnostics: DiagnosticCollector,
   operation: Operation,
-  responses: ResponseIndex,
   responseType: Type,
-  parentDescription?: string,
-) {
+  parentDescription: string | undefined,
+): ProcessedResponseType {
   // Get body
   const verb = getOperationVerb(program, operation);
   let { body: resolvedBody, metadata } = diagnostics.pipe(
@@ -171,35 +230,14 @@ function processResponseType(
     }
   }
 
-  // Put them into currentEndpoint.responses
-  for (const statusCode of statusCodes) {
-    // the first model for this statusCode/content type pair carries the
-    // description for the endpoint. This could probably be improved.
-    const response: HttpOperationResponse = responses.get(statusCode) ?? {
-      statusCodes: statusCode,
-      type: responseType,
-      description: getResponseDescription(
-        program,
-        operation,
-        responseType,
-        statusCode,
-        metadata,
-        parentDescription,
-      ),
-      responses: [],
-    };
-
-    if (resolvedBody !== undefined) {
-      response.responses.push({
-        body: resolvedBody,
-        headers,
-        properties: metadata,
-      });
-    } else {
-      response.responses.push({ headers, properties: metadata });
-    }
-    responses.set(statusCode, response);
-  }
+  return {
+    statusCodes: statusCodes,
+    type: responseType,
+    parentDescription,
+    body: resolvedBody,
+    headers: headers,
+    properties: metadata,
+  };
 }
 
 /**
@@ -290,39 +328,53 @@ function isPlainResponseBody(program: Program, type: Type): boolean {
   return !result || !result.metadata.some((p) => p.kind !== "bodyProperty");
 }
 
-function getResponseDescription(
+function getResponsesDescription(
   program: Program,
   operation: Operation,
-  responseType: Type,
   statusCode: HttpStatusCodes[number],
-  metadata: HttpProperty[],
-  parentDescription?: string,
-): string | undefined {
-  // If a parent union provided a description, use that first
-  if (parentDescription) {
-    return parentDescription;
+  variants: ProcessedResponseType[],
+) {
+  if (variants.length <= 0) {
+    return getStatusCodeDescription(statusCode);
   }
 
-  // NOTE: If the response type is an envelope and not the same as the body
-  // type, then use its @doc as the response description. However, if the
-  // response type is the same as the body type, then use the default status
-  // code description and don't duplicate the schema description of the body
-  // as the response description. This allows more freedom to change how
-  // TypeSpec is expressed in semantically equivalent ways without causing
-  // the output to change unnecessarily.
-  if (isResponseEnvelope(metadata)) {
-    const desc = getDoc(program, responseType);
-    if (desc) {
-      return desc;
+  function getSingleResponseDescription(variant: ProcessedResponseType): string | undefined {
+    if (variant.parentDescription) {
+      return variant.parentDescription;
+    }
+
+    // NOTE: If the response type includes response envelope metadata (e.g. @statusCode, @header),
+    // then use its @doc as the response description. Plain body types intentionally fall back to
+    // the status-code/operation-level descriptions to avoid duplicating the schema description as
+    // the response description.
+    if (isResponseEnvelope(variant.properties)) {
+      const desc = getDoc(program, variant.type);
+      if (desc) return desc;
     }
+
+    return undefined;
   }
 
-  const desc = isErrorModel(program, responseType)
-    ? getErrorsDoc(program, operation)
-    : getReturnsDoc(program, operation);
-  if (desc) {
-    return desc;
+  const firstDesc = getSingleResponseDescription(variants[0]);
+  if (firstDesc && variants.every((v) => getSingleResponseDescription(v) === firstDesc)) {
+    return firstDesc;
   }
 
-  return getStatusCodeDescription(statusCode);
+  let hasError = false,
+    hasSuccess = false;
+  for (const variant of variants) {
+    if (isErrorModel(program, variant.type)) {
+      hasError = true;
+    } else {
+      hasSuccess = true;
+    }
+  }
+
+  let desc: string | undefined;
+  if (hasSuccess && !hasError) {
+    desc = getReturnsDoc(program, operation);
+  } else if (hasError && !hasSuccess) {
+    desc = getErrorsDoc(program, operation);
+  }
+  return desc || getStatusCodeDescription(statusCode);
 }