Feedback from code review

Author: JoyceZhu

.github/actions/file/action.yml

@@ -14,9 +14,10 @@ inputs:
   cached_filings:
     description: "Cached filings from previous runs, as stringified JSON. Without this, duplicate issues may be filed."
     required: false
-  open_tracking_issues:
-    description: "In the 'file' step, also open tracking issues which link to all issues with the same problem"
+  open_grouped_issues:
+    description: "In the 'file' step, also open tracking issues which link to all issues with the same root cause"
     required: false
+    default: "false"
 
 outputs:
   filings:

.github/actions/file/src/index.ts

@@ -1,4 +1,10 @@
-import type { Finding, ResolvedFiling, RepeatedFiling } from "./types.d.js";
+import type {
+  Finding,
+  ResolvedFiling,
+  RepeatedFiling,
+  FindingGroupIssue,
+  Filing,
+} from "./types.d.js";
 import process from "node:process";
 import core from "@actions/core";
 import { Octokit } from "@octokit/core";
@@ -11,7 +17,7 @@ import { isResolvedFiling } from "./isResolvedFiling.js";
 import { openIssue } from "./openIssue.js";
 import { reopenIssue } from "./reopenIssue.js";
 import { updateFilingsWithNewFindings } from "./updateFilingsWithNewFindings.js";
-import { OctokitResponse } from "@octokit/types";
+import type { OctokitResponse } from "@octokit/types";
 const OctokitWithThrottling = Octokit.plugin(throttling);
 
 export default async function () {
@@ -24,7 +30,7 @@ export default async function () {
   const cachedFilings: (ResolvedFiling | RepeatedFiling)[] = JSON.parse(
     core.getInput("cached_filings", { required: false }) || "[]",
   );
-  const shouldOpenTrackingIssues = core.getBooleanInput("open_tracking_issues");
+  const shouldOpenGroupedIssues = core.getBooleanInput("open_grouped_issues");
   core.debug(`Input: 'findings: ${JSON.stringify(findings)}'`);
   core.debug(`Input: 'repository: ${repoWithOwner}'`);
   core.debug(`Input: 'cached_filings: ${JSON.stringify(cachedFilings)}'`);
@@ -55,8 +61,7 @@ export default async function () {
   const filings = updateFilingsWithNewFindings(cachedFilings, findings);
 
   // Track new issues for grouping
-  const newIssuesByProblemShort: Record<string, { url: string; id: number }[]> =
-    {};
+  const newIssuesByProblemShort: Record<string, FindingGroupIssue[]> = {};
   const trackingIssueUrls: Record<string, string> = {};
 
   for (const filing of filings) {
@@ -69,12 +74,13 @@ export default async function () {
       } else if (isNewFiling(filing)) {
         // Open a new issue for the filing
         response = await openIssue(octokit, repoWithOwner, filing.findings[0]);
-        (filing as any).issue = { state: "open" } as Issue;
+        (filing as Filing).issue = { state: "open" } as Issue;
         // Track for grouping
-        if (shouldOpenTrackingIssues) {
+        if (shouldOpenGroupedIssues) {
           const problemShort: string = filing.findings[0].problemShort;
-          if (!newIssuesByProblemShort[problemShort])
+          if (!newIssuesByProblemShort[problemShort]) {
             newIssuesByProblemShort[problemShort] = [];
+          }
           newIssuesByProblemShort[problemShort].push({
             url: response.data.html_url,
             id: response.data.number,
@@ -102,12 +108,12 @@ export default async function () {
   }
 
   // Open tracking issues for each root cause and link back from each newly-created issue
-  if (shouldOpenTrackingIssues) {
+  if (shouldOpenGroupedIssues) {
     for (const [problemShort, issues] of Object.entries(
       newIssuesByProblemShort,
-    ) as [string, { url: string; id: number }[]][]) {
+    )) {
       if (issues.length > 1) {
-        const title: string = `${problemShort} issues`;
+        const title: string = `Accessibility tracking issue for all ${problemShort} issues`;
         const body: string =
           `# ${problemShort} issues\n\n` +
           issues.map((issue) => `- [ ] ${issue.url}`).join("\n");
@@ -133,9 +139,9 @@ export default async function () {
         }
       }
     }
-
-    core.setOutput("filings", JSON.stringify(filings));
-    core.debug(`Output: 'filings: ${JSON.stringify(filings)}'`);
-    core.info("Finished 'file' action");
   }
+
+  core.setOutput("filings", JSON.stringify(filings));
+  core.debug(`Output: 'filings: ${JSON.stringify(filings)}'`);
+  core.info("Finished 'file' action");
 }

.github/actions/file/src/types.d.ts

@@ -33,3 +33,8 @@ export type RepeatedFiling = {
 };
 
 export type Filing = ResolvedFiling | NewFiling | RepeatedFiling;
+
+export type FindingGroupIssue = {
+  url: string;
+  id: number;
+};

action.yml

@@ -31,7 +31,7 @@ inputs:
     description: "Whether to skip assigning filed issues to Copilot"
     required: false
     default: "false"
-  open_tracking_issues:
+  open_grouped_issues:
     description: "In the 'file' step, also open tracking issues which link to all issues with the same problem"
     required: false
     default: "false"
@@ -92,7 +92,7 @@ runs:
         repository: ${{ inputs.repository }}
         token: ${{ inputs.token }}
         cached_filings: ${{ steps.normalize_cache.outputs.value }}
-        open_tracking_issues: ${{ inputs.open_tracking_issues }}
+        open_grouped_issues: ${{ inputs.open_grouped_issues }}
     - if: ${{ steps.file.outputs.filings }}
       name: Get issues from filings
       id: get_issues_from_filings