Skip to content

Commit 850435d

Browse files
refactor: delete unused symbols (#766)
* refactor: delete unused symbols * docs(contracts): drop the sentence about deriving custom rule ids
1 parent 13b2eb4 commit 850435d

4 files changed

Lines changed: 5 additions & 68 deletions

File tree

‎packages/triage-contracts/README.md‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ and trigger mail. Every app and most packages import it.
99

1010
### Policy
1111

12-
A policy sets which rules run on a repository's pull requests and what happens after. Its rules are stored under `checks` (built-in) and `custom`. `GET /api/integrations/pack-schema` returns its JSON Schema as `checkPack`, the part of a policy file that extends another repository's file as `policyOverride`, the repository settings as `repoPolicy`, and `.corbits-triage.yml` as `policyFile`: schema 3, a list of `rules` (each `text`, `check`, `reason` and `evidence`), `actions` (`on` and steps per verdict) and `labels`, which `readPolicyFile` maps onto the stored shape; `checkPolicyFile` answers the same or each problem with its line, and the file's v4 keys as `workflow`. Those keys (`policy-workflow.ts`) are `groups` (each `association`, `teams` or `users`), one key per block, `code-review` (`on`, and optionally `for`, `when: { triage, paths, rules }` and `provider`; a file setting the retired `clean` is refused with a pointer to a step's `when.findings`), with `security` and `deslop` reserved: `checkPolicyFile` reads a file without them and without any `decide` rule with a condition on one, names each as `pending`, and refuses the file as written if the rest is refused too; `patchPolicyFile` refuses a file still holding them unless the batch has `strip-reserved`, which removes them. The other v4 keys are `pace`, `pauseLabel` and `decide` (rules `{ if, do }`, `if` on `all`, `triage`, `event`, `author` and each block's outcome); they are validated and returned as written, and nothing acts on them yet except steps naming a group and, on GitHub, `pauseLabel`, whose label on a pull request stops the hub starting runs for it. A file that sets `extends` holds none of them. Groups are matched first match wins, in the file's order: an author is in at most one of the file's groups, then in each built-in group (`maintainers`, `external`, `first-timers`, `bots`) their author kind puts them in, which are facts matched independently, then in `everyone`, which always matches; a file group with a built-in group's name replaces it. `authorGroupsOf(groups, author)` answers that set, failing closed: it answers none, so no `{ group }` but `everyone` holds, when the author's kind is not known or teams that can't be read would decide their file group. A step's built-in group reads as author kinds, so it still holds when teams can't be read, and is not known only when the author's kind is. The teams read for an author are every team the policy names, in a step or a group (`policyTeams`). A step's `when.author` names groups as the policy holds them, `{ group }` for a file group or `everyone` and a built-in group's kinds otherwise, and holds when the author's set includes it; approving or merging must never rest on `everyone` alone. `{ all: clean }` holds when at least one block ran and every block that ran is clean; skipped blocks never count, so it fails when every block was skipped or none is on. A rule that approves or merges without `triage` matches only Pass (`ruleVerdicts`). Incomplete is never clean: a rule that approves or merges can't name `incomplete`. A file that sets `actions` and no `decide` gets its `decide` from `decideOf`: one rule `{ if: { triage: <verdict> }, do: <its steps> }` per verdict with steps, `do: nothing` for a Pass or Fail with none, and no rule for an Unsure with none, each limited to `event: <actions.on>` when the actions wake on fewer than every event, every step as written; a file that extends another has no `decide`. A `decide` rule can't use `review` yet. A file setting any v4 key is read as schema 3 and needs `rules`. Problems name their line from the YAML parser's node positions (`yaml`, core schema, aliases and the keys `__proto__`, `constructor` and `prototype` refused); schema errors are said in the file's terms by `policy-file-problems.ts`. A file without `rules` or a v4 key is read as a policy or override without `kind`, `schemaVersion` and `repo`. Constraints the schema cannot state are checked when the policy is read: a custom rule's `options` and `failOn` must not repeat and `failOn` must be among `options`, a score's `failBelow` must be one of its levels after the first, and a custom id must not be a built-in rule's or read as a judge question's. A rule that fails any of these rejects the policy with its reason. Custom rules are decision-model questions (`kind: "model"`): `is-true`, `score` or `choose`, or text rules (`kind: "text"`) that fail when any of `fields` (`title`, `description`, `commits`) contains one of `phrases`, or with `failWhen: "not-contains"` when one contains none, ignoring case; a stored rule of the retired `kind: "rule"` rejects the policy. The reader ignores unknown built-in rows. Built-in rule ids renamed earlier (`size`, `issue`, `conflicts`, `ci`, `paths`, `draft`, `review`) are still read and mapped to their new ids by `RENAMED_RULE_IDS`; an old `size` row loads as both size limits. A new custom rule's id is its name as lowercase words joined by hyphens, numbered past any taken id; it never starts with `docs-` or `focused-` or ends in `-file`, `-hunk` or `-slice` and numbers, as judge question ids do.
12+
A policy sets which rules run on a repository's pull requests and what happens after. Its rules are stored under `checks` (built-in) and `custom`. `GET /api/integrations/pack-schema` returns its JSON Schema as `checkPack`, the part of a policy file that extends another repository's file as `policyOverride`, the repository settings as `repoPolicy`, and `.corbits-triage.yml` as `policyFile`: schema 3, a list of `rules` (each `text`, `check`, `reason` and `evidence`), `actions` (`on` and steps per verdict) and `labels`, which `readPolicyFile` maps onto the stored shape; `checkPolicyFile` answers the same or each problem with its line, and the file's v4 keys as `workflow`. Those keys (`policy-workflow.ts`) are `groups` (each `association`, `teams` or `users`), one key per block, `code-review` (`on`, and optionally `for`, `when: { triage, paths, rules }` and `provider`; a file setting the retired `clean` is refused with a pointer to a step's `when.findings`), with `security` and `deslop` reserved: `checkPolicyFile` reads a file without them and without any `decide` rule with a condition on one, names each as `pending`, and refuses the file as written if the rest is refused too; `patchPolicyFile` refuses a file still holding them unless the batch has `strip-reserved`, which removes them. The other v4 keys are `pace`, `pauseLabel` and `decide` (rules `{ if, do }`, `if` on `all`, `triage`, `event`, `author` and each block's outcome); they are validated and returned as written, and nothing acts on them yet except steps naming a group and, on GitHub, `pauseLabel`, whose label on a pull request stops the hub starting runs for it. A file that sets `extends` holds none of them. Groups are matched first match wins, in the file's order: an author is in at most one of the file's groups, then in each built-in group (`maintainers`, `external`, `first-timers`, `bots`) their author kind puts them in, which are facts matched independently, then in `everyone`, which always matches; a file group with a built-in group's name replaces it. `authorGroupsOf(groups, author)` answers that set, failing closed: it answers none, so no `{ group }` but `everyone` holds, when the author's kind is not known or teams that can't be read would decide their file group. A step's built-in group reads as author kinds, so it still holds when teams can't be read, and is not known only when the author's kind is. The teams read for an author are every team the policy names, in a step or a group (`policyTeams`). A step's `when.author` names groups as the policy holds them, `{ group }` for a file group or `everyone` and a built-in group's kinds otherwise, and holds when the author's set includes it; approving or merging must never rest on `everyone` alone. `{ all: clean }` holds when at least one block ran and every block that ran is clean; skipped blocks never count, so it fails when every block was skipped or none is on. A rule that approves or merges without `triage` matches only Pass (`ruleVerdicts`). Incomplete is never clean: a rule that approves or merges can't name `incomplete`. A file that sets `actions` and no `decide` gets its `decide` from `decideOf`: one rule `{ if: { triage: <verdict> }, do: <its steps> }` per verdict with steps, `do: nothing` for a Pass or Fail with none, and no rule for an Unsure with none, each limited to `event: <actions.on>` when the actions wake on fewer than every event, every step as written; a file that extends another has no `decide`. A `decide` rule can't use `review` yet. A file setting any v4 key is read as schema 3 and needs `rules`. Problems name their line from the YAML parser's node positions (`yaml`, core schema, aliases and the keys `__proto__`, `constructor` and `prototype` refused); schema errors are said in the file's terms by `policy-file-problems.ts`. A file without `rules` or a v4 key is read as a policy or override without `kind`, `schemaVersion` and `repo`. Constraints the schema cannot state are checked when the policy is read: a custom rule's `options` and `failOn` must not repeat and `failOn` must be among `options`, a score's `failBelow` must be one of its levels after the first, and a custom id must not be a built-in rule's or read as a judge question's. A rule that fails any of these rejects the policy with its reason. Custom rules are decision-model questions (`kind: "model"`): `is-true`, `score` or `choose`, or text rules (`kind: "text"`) that fail when any of `fields` (`title`, `description`, `commits`) contains one of `phrases`, or with `failWhen: "not-contains"` when one contains none, ignoring case; a stored rule of the retired `kind: "rule"` rejects the policy. The reader ignores unknown built-in rows. Built-in rule ids renamed earlier (`size`, `issue`, `conflicts`, `ci`, `paths`, `draft`, `review`) are still read and mapped to their new ids by `RENAMED_RULE_IDS`; an old `size` row loads as both size limits.
1313

1414
`actions` holds one `when` (the events it wakes on) and an ordered list of steps per verdict. Each step is `{ do, target?, when? }`; its `when` holds conditions that must all hold for it to run (`whenHolds`): `author` (any of GitHub's `author_association` values `OWNER`, `MEMBER`, `COLLABORATOR`, `CONTRIBUTOR`, `FIRST_TIME_CONTRIBUTOR`, `FIRST_TIMER`, `NONE`, `MANNEQUIN`, or `BOT` for a bot whatever its association, `{ team }`, `{ users }`; a policy naming a group of `AUTHOR_GROUPS`, `maintainers`, `external`, `first-timers` or `bots`, reads it as exactly that group's kinds), `linkedIssue`, `ci` (any of `passing`, `failing`, `pending`, `none`), `files` and `lines` as `{ over?, atMost? }`, and `findings` as `{ above: P0–P3 }` or `{ any: boolean }`, which `whenHolds` leaves to the manager once code review reports and a policy file accepts only with `code-review` on. Every rule row may carry `text`, `reason` and `evidence` (`#44`, `owner/name#44`, https links or `maintainer-stated`). An action runs on its own only when the workspace's `pr-triage` system role holds `invoke` on `triage-do:<kind>:<owner>/<name>`; otherwise it waits for a person holding that grant, and `close` on `unsure` and `merge` on anything but `pass` always wait. `labels` names the label the mirror writes for each verdict; a verdict it leaves out keeps its default (`Triage: Ready`, `Triage: Not Ready`, `Triage: Needs Human Review`). Repository roles were removed, so a step whose target is a role is left out and named in the pack's `dropped`.
1515

‎packages/triage-contracts/src/check-pack.test.ts‎

Lines changed: 2 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1,17 +1,14 @@
11
import { type } from "arktype";
22
import { describe, expect, test } from "bun:test";
33
import {
4-
applyRecommended,
54
catalogCheckEnabled,
65
checkPackName,
76
checkPackSchema,
8-
customRuleId,
97
defaultActions,
108
emptyPack,
119
parseCheckPack,
1210
readCheckPack,
1311
recommendedPack,
14-
type CheckPack,
1512
type CustomCheck,
1613
type PolicyActions,
1714
} from "./check-pack.ts";
@@ -106,23 +103,13 @@ describe("check pack contract", () => {
106103
});
107104

108105
test("a custom rule's id never reads as a judge question's id", () => {
109-
for (const [name, id] of [["Docs sync", "rule-docs-sync"], ["Focused slice 001", "rule-focused-slice-001-rule"], ["Config file 1", "config-file-1-rule"], ["Schema hunk", "schema-hunk-rule"], ["Lockfile", "lockfile"]]) {
110-
expect(customRuleId(name!, [])).toBe(id!);
111-
}
112-
expect(customRuleId("Schema hunk", ["schema-hunk-rule"])).toBe("schema-hunk-rule-2");
113106
for (const id of ["docs-file-001", "focused-slice-001", "duplicates-open-pull-request-001", "ticket-file-001", "ticket-hunk-001-002", "ticket-slice-2"]) {
114107
expect(() => readCheckPack({ ...emptyPack(REPO), custom: [{ ...CUSTOM[0]!, id }] })).toThrow(`Custom rule ${id} id must not start with docs-, focused- or duplicates-open-pull-request- or end in -file, -hunk or -slice and numbers.`);
115108
}
116109
});
117110

118-
test("a custom rule's id is its name in lowercase words, numbered past taken and built-in ids", () => {
119-
expect(customRuleId("Ticket in the title!", [])).toBe("ticket-in-the-title");
120-
expect(customRuleId("Ticket in the title", ["ticket-in-the-title", "ticket-in-the-title-2"])).toBe("ticket-in-the-title-3");
121-
expect(customRuleId("CI", [])).toBe("ci-2");
122-
expect(customRuleId("Focused", [])).toBe("rule-focused");
123-
expect(customRuleId("Ticket", ["ticket-4"])).toBe("ticket-5");
124-
expect(customRuleId("…", [])).toBe("rule");
125-
const named = { ...CUSTOM[0]!, id: customRuleId("Ticket", []) };
111+
test("a custom rule's id is checked against built-in ids and the id format", () => {
112+
const named = { ...CUSTOM[0]!, id: "ticket" };
126113
expect(readCheckPack({ ...emptyPack(REPO), custom: [named] }).custom).toEqual([named]);
127114
expect(() => readCheckPack({ ...emptyPack(REPO), custom: [{ ...named, id: "ci-green" }] })).toThrow("Custom rule ci-green id must not be a built-in rule's id.");
128115
expect(() => readCheckPack({ ...emptyPack(REPO), custom: [{ ...named, id: "size" }] })).toThrow("Custom rule size id must not be a built-in rule's id.");
@@ -249,14 +236,4 @@ describe("check pack contract", () => {
249236
expect(checkPackSchema({ ...emptyPack(REPO), actions: { ...ACTIONS, always: [] } })).toBeInstanceOf(type.errors);
250237
expect(() => readCheckPack({ ...emptyPack(REPO), schemaVersion: 3 })).toThrow("schemaVersion must be 2.");
251238
});
252-
253-
test("Use recommended keeps existing custom", () => {
254-
const started: CheckPack = {
255-
...emptyPack(REPO),
256-
custom: CUSTOM,
257-
};
258-
const next = applyRecommended(started);
259-
expect(next.checks["max-files-changed"]?.enabled).toBe(true);
260-
expect(next.custom).toEqual(started.custom);
261-
});
262239
});

‎packages/triage-contracts/src/check-pack.ts‎

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -335,7 +335,6 @@ export type CustomCheck = ModelCustomCheck | TextCustomCheck | PatternsCustomChe
335335

336336
const LEVELS_MIN = 2;
337337
const LEVELS_MAX = 10;
338-
export const DEFAULT_SCORE_LEVELS = ["poor", "fair", "good", "great"] as const;
339338
const GLOB_MAX = 200;
340339
const GLOBS_MAX = 50;
341340
const CUSTOM_ID = /^[a-z0-9]+(?:-[a-z0-9]+)*$/;
@@ -393,8 +392,6 @@ export const LABEL_MAX = 50;
393392
const LIST_MAX = 50;
394393
const STEPS_MAX = 20;
395394

396-
export type CheckRef = string;
397-
398395
/** The events a policy's actions wake on; a catch-up run wakes them whatever its policy names. */
399396
export const POLICY_EVENTS = ["opened", "updated", "checks", "ready"] as const satisfies readonly TriageEvent[];
400397
export type PolicyEvent = (typeof POLICY_EVENTS)[number];
@@ -627,21 +624,6 @@ function customIdProblem(id: string): string | undefined {
627624
return undefined;
628625
}
629626

630-
/**
631-
* A custom rule's id from its name: lowercase words joined by hyphens. A taken name is numbered past the highest
632-
* number its ids already carry, so the id of a removed rule is not handed to another while a later one remains.
633-
*/
634-
export function customRuleId(name: string, taken: Iterable<string>): string {
635-
let base = name.toLowerCase().replace(/[^a-z0-9]+/g, "-").slice(0, CUSTOM_ID_MAX - 12).replace(/^-+|-+$/g, "") || "rule";
636-
if (/^(?:docs|focused|duplicates-open-pull-request)-/.test(`${base}-`)) base = `rule-${base}`;
637-
if (QUESTION_LIKE.test(`${base}-2`)) base = `${base}-rule`;
638-
const ids = [...taken];
639-
const numbered = new RegExp(`^${base}-(\\d+)$`);
640-
const highest = Math.max(0, ...ids.flatMap((id) => numbered.exec(id)?.[1] ?? []).map(Number));
641-
if (highest === 0 && !ids.includes(base) && !RESERVED_IDS.has(base)) return base;
642-
return `${base}-${Math.max(highest, 1) + 1}`;
643-
}
644-
645627
const TRACKER_SET = new Set<string>(ISSUE_TRACKERS);
646628

647629
export const CHECK_PACK_TITLE_PREFIX = "check-pack/";
@@ -711,10 +693,6 @@ export function starterPack(repo: string): CheckPack {
711693
return { ...emptyPack(repo), checks, actions: { when: [...POLICY_EVENTS], pass: [], fail: [], unsure: [] } };
712694
}
713695

714-
export function applyRecommended(pack: CheckPack): CheckPack {
715-
return { ...pack, checks: recommendedPack(pack.repo).checks };
716-
}
717-
718696
export function catalogCheckEnabled(pack: CheckPack, id: CatalogId): boolean {
719697
const row = pack.checks[id];
720698
return row !== undefined && row.enabled !== false;

‎packages/triage-workflows/src/logic/facts.ts‎

Lines changed: 2 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
1-
import { checkCancelled, checkFailed, checkPending, latestDecisive, listPrFilesPage, readPrTemplates, reviewedOnHead, summarizeChecks, type CheckRun, type GithubFetch, type IssueComment, type Review } from "@corbits/github-reads";
2-
import { authorAssociation, authorKindOf, catalogCheckEnabled, isGeneratedPath, policyTeams, shownBody, teamSlug, tierOf, visibleText, type CheckPack, type RepoPolicy, type RepoSource, type TriageEvent } from "@corbits/triage-contracts";
1+
import { checkCancelled, checkFailed, checkPending, latestDecisive, reviewedOnHead, summarizeChecks, type CheckRun, type IssueComment, type Review } from "@corbits/github-reads";
2+
import { authorAssociation, authorKindOf, isGeneratedPath, policyTeams, shownBody, teamSlug, tierOf, visibleText, type CheckPack, type RepoPolicy, type RepoSource, type TriageEvent } from "@corbits/triage-contracts";
33
import { hiddenIn, hiddenMarkup, nothingRemoved, writtenDescription } from "./hidden-content.js";
44
import type { PrFacts, PrFileFacts } from "./checks.js";
55
import { MAX_CODE_RULES_PATCH_CHARS } from "./quality.js";
@@ -146,19 +146,6 @@ export function addFilesPage(listed: ListedFiles, cursor: FilesCursor, page: Fil
146146
return { offset: page.next, ...next };
147147
}
148148

149-
/** Every changed file of a pull request, read page by page as the facts director reads them. */
150-
export async function listFiles(gh: GithubFetch, repo: string, number: number): Promise<PrFileFacts[]> {
151-
const listed: ListedFiles = { files: [], patchChars: 0 };
152-
let cursor: FilesCursor | undefined = FIRST_FILES_PAGE;
153-
while (cursor !== undefined) {
154-
const page = await listPrFilesPage(gh, repo, number, cursor);
155-
const next = addFilesPage(listed, cursor, { ...page, files: page.files.flatMap(fileFacts) });
156-
if (typeof next === "string") throw new Error(`${next} for #${number}`);
157-
cursor = next;
158-
}
159-
return listed.files;
160-
}
161-
162149
function optionalString(value: unknown): string | undefined {
163150
return typeof value === "string" && value.length > 0 ? value : undefined;
164151
}
@@ -251,11 +238,6 @@ export interface ItemContext {
251238
source: RepoSource;
252239
}
253240

254-
/** A GitHub repository's pull request templates, read only when the hidden-content rule tells their comments apart. */
255-
export async function templatesFor(gh: GithubFetch, repo: string, pack: CheckPack): Promise<string[]> {
256-
return catalogCheckEnabled(pack, "hidden-content") ? readPrTemplates(gh, repo) : [];
257-
}
258-
259241
/** `text` as a reader sees it, noting in `hidden` what it hid; `where` names the place from its shown text. */
260242
function shownText(text: string, hidden: string[], where: (shown: string) => string): string {
261243
const removed = nothingRemoved();

0 commit comments

Comments
 (0)