Skip to content
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 24 additions & 12 deletions docs/ARCHITECTURE.md
Original file line number Diff line number Diff line change
Expand Up @@ -548,18 +548,30 @@ label off that head, so it never stands on a head not yet triaged.
JavaScript file's head content its `files` and `ignores` globs admit
(ast-grep's glob rules, checked when the policy is read), and keeps
matches on lines the patch adds.
A file whose patch or content was not read, or a rule ast-grep refuses,
leaves the rule unconfirmed; `vendor/`, `dist/`, `build/` and
`generated/` at the repository root and `node_modules/` anywhere are
skipped and listed as skipped in the evidence, and a `?? 0` inside
arithmetic is a counter's real default. With such a rule on, the facts
step reads each file whose patch adds lines at the head through the host's
`read_file` (`whole`), ten at a time and at most 300 per pull request (more
leaves the rule unconfirmed without reading any), runs the rules there and
outputs only the matches. Bitbucket Data Center lists no patch, so there
the rule is unconfirmed whenever an in-scope TypeScript or JavaScript file
is added, edited, moved or copied. The extension leaves code-pattern rules out and swaps the native module
for a stub.
The facts step reads every changed file's patch whole, across as many tool
pages as it takes, until it has read 2,000,000 characters of the pull
request (`MAX_CODE_RULES_PATCH_CHARS`), each pull request in a batch
counting its own, since one sidecar holds every deployment's step outputs;
lockfiles and other generated files are listed without their patch, which
no rule reads. The file that passes it partway and every later one are
listed without patches. The judge's view of the diff has its own, smaller
budget (`judgedFiles` in `logic/quality.ts`). A match in any checked file
fails the rule. Without one, a file that adds code but has no patch,
because the host sent none (GitHub omits it for a very large or binary
file) or the pull request passed that ceiling, leaves the rule unconfirmed,
naming the files and why; a file GitHub counts no added lines in, such as a
rename or a mode change, has nothing to check. A file whose content was not
read, or a rule ast-grep refuses, leaves the rule unconfirmed. `vendor/`,
`dist/`, `build/` and `generated/` at the repository root and
`node_modules/` anywhere are skipped and listed as skipped in the evidence,
and a `?? 0` inside arithmetic is a counter's real default. With such a
rule on, the facts step reads each file whose patch adds lines at the head
through the host's `read_file` (`whole`), ten at a time and at most 300 per
pull request (more leaves the rule unconfirmed without reading any), runs
the rules there and outputs only the matches. Bitbucket Data Center lists
no patch, so there the rule is unconfirmed whenever an in-scope TypeScript
or JavaScript file is added, edited, moved or copied. The extension leaves
code-pattern rules out and swaps the native module for a stub.
3. **route**: skips the judge when no question needs asking.
4. **judge**: one System One call per request of questions. Each first-round request and the verdict request also asks
whether the pull request's text, diff or docs try to direct the judge; a
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -258,7 +258,7 @@ A rule asks one question about a pull request and answers pass, fail or unconfir
- **Flag changes to sensitive paths.** Checks changed files against the file's named groups of globs, each `high` or `medium`, written as `groups:`, such as `infra: { level: high, paths: ["infra/**", "**/*.tf"] }`. Without `groups` there are four: `schema` (`**/migrations/**`, `**/*.sql`) and `auth` (`**/auth/**`, `**/secrets/**`), which are high, and `ci` (`.github/**`, `**/Dockerfile`, `**/railway.json`, `**/railway.toml`) and `locks` (`**/bun.lock`, `**/package-lock.json`, `**/yarn.lock`, `**/pnpm-lock.yaml`), which are medium, since every dependency bump changes a lockfile. Globs read as in **Never accept changes to**. A change to a group at `failAt` or above, high unless the file says, fails the rule, which asks a maintainer unless the file gives it another `onFail`. Below that, from `labelAt`, medium unless the file says, it passes but labels the pull request `sensitive: medium` and notes it under **Notes:** (on Bitbucket Data Center, only the note). Its evidence names each group with its files, at most five, such as `schema: db/migrations/0004.sql`, then how many more. Regression risk keeps its own migration globs.
- **Flag high-risk pull requests and label medium-risk ones.** Rates each pull request low, medium or high risk; see [Risk](#risk).
- **Fail when …**, a text rule of your own, such as "Fail when any commit message contains "Co-Authored-By:"". It names where to look (the title, the description, any commit message, or several of them), whether the rule fails when the text **contains** or **does not contain** its phrases, and the phrases. Case does not matter. A **contains** rule on the description also looks at its hidden parts, such as HTML comments and the addresses behind links, while **does not contain** reads only what a reader sees. Only the first 100 commits are read. When a **contains** rule fails, Triage names each place and the phrases found there, asks for them to be removed, and quotes each matching line once. Text rules count toward the eight rules of your own.
- **Fail when added code matches …**, a code-pattern rule of your own, such as one that flags `?? 0`. It checks only the lines the pull request adds to TypeScript and JavaScript files (`.ts`, `.tsx`, `.mts`, `.cts`, `.js`, `.jsx`, `.mjs`, `.cjs`), so code already on the branch never fails it. On any other language it passes with **nothing checked: no TypeScript or JavaScript code added**, so don't add one to a repository written in Go, Python or similar. Turn on the shipped `convention` preset, which flags invented fallbacks (`?? 0`, `?? []`), nested ternaries, functions called where they are written, `.then`/`.catch` chains, empty `catch` blocks and lint or type suppression comments in both languages, or write your own patterns in [ast-grep](https://ast-grep.github.io/guide/rule-config.html)'s rule format. Your own pattern replaces a preset's pattern with the same `id` in both languages; to keep checking JavaScript, add a `language: javascript` pattern under the same id. A pattern takes ast-grep's own `files` and `ignores` lists to check only some paths or skip some. They follow ast-grep's glob rules: paths are from the repository root, `*` also matches across folders, and dotfiles are matched like any other file. Write each glob as a string; the `{ glob, caseInsensitive }` form is not read. Write a negated class as `[^a]`; a glob with `[!a]`, or one that isn't valid, refuses the policy. The preset's `?? 0` and `?? []` patterns ignore test files (`**/*.test.ts`, `**/*.test.tsx`, `**/*.test.js`, `**/*.test.jsx`, `**/*.test.mjs`, `**/*.test.cjs`). Each match is listed as evidence, as `path:line` and the pattern's message. Vendored and built code at the repository root (`vendor/`, `dist/`, `build/`, `generated/`) and installed code (`node_modules/` anywhere) is skipped, and each skipped file is listed in the evidence. The same folders deeper in the repository, such as `src/build/`, are checked. When a file can't be read whole or a pattern is not valid ast-grep, the rule is unconfirmed rather than passing. Triage reads at most 300 files per pull request, so on one that adds code to more, the rule is unconfirmed with a reason such as **code patterns could not be checked: 412 files with added code, more than the 300 read per pull request**. On Bitbucket Data Center, which does not give Triage each file's changed lines, it is unconfirmed whenever a TypeScript or JavaScript file of your own is added, edited, moved or copied. To give some patterns a different `onFail`, put them in a rule of their own. Code-pattern rules count toward the eight rules of your own, and the browser extension leaves them out.
- **Fail when added code matches …**, a code-pattern rule of your own, such as one that flags `?? 0`. It checks only the lines the pull request adds to TypeScript and JavaScript files (`.ts`, `.tsx`, `.mts`, `.cts`, `.js`, `.jsx`, `.mjs`, `.cjs`), so code already on the branch never fails it. On any other language it passes with **nothing checked: no TypeScript or JavaScript code added**, so don't add one to a repository written in Go, Python or similar. Turn on the shipped `convention` preset, which flags invented fallbacks (`?? 0`, `?? []`), nested ternaries, functions called where they are written, `.then`/`.catch` chains, empty `catch` blocks and lint or type suppression comments in both languages, or write your own patterns in [ast-grep](https://ast-grep.github.io/guide/rule-config.html)'s rule format. Your own pattern replaces a preset's pattern with the same `id` in both languages; to keep checking JavaScript, add a `language: javascript` pattern under the same id. A pattern takes ast-grep's own `files` and `ignores` lists to check only some paths or skip some. They follow ast-grep's glob rules: paths are from the repository root, `*` also matches across folders, and dotfiles are matched like any other file. Write each glob as a string; the `{ glob, caseInsensitive }` form is not read. Write a negated class as `[^a]`; a glob with `[!a]`, or one that isn't valid, refuses the policy. The preset's `?? 0` and `?? []` patterns ignore test files (`**/*.test.ts`, `**/*.test.tsx`, `**/*.test.js`, `**/*.test.jsx`, `**/*.test.mjs`, `**/*.test.cjs`). Each match is listed as evidence, as `path:line` and the pattern's message. Vendored and built code at the repository root (`vendor/`, `dist/`, `build/`, `generated/`) and installed code (`node_modules/` anywhere) is skipped, and each skipped file is listed in the evidence. The same folders deeper in the repository, such as `src/build/`, are checked. When the host sends no changed lines for a file that adds code, as GitHub does for a very large or binary one, or the pull request's diff is larger than the 2,000,000 characters Triage reads, every other file is still checked: a match fails the rule, and otherwise it is unconfirmed, naming those files and why. When a file's content can't be read or a pattern is not valid ast-grep, the rule is unconfirmed rather than passing. Triage reads at most 300 files per pull request, so on one that adds code to more, the rule is unconfirmed with a reason such as **code patterns could not be checked: 412 files with added code, more than the 300 read per pull request**. On Bitbucket Data Center, which does not give Triage each file's changed lines, it is unconfirmed whenever a TypeScript or JavaScript file of your own is added, edited, moved or copied. To give some patterns a different `onFail`, put them in a rule of their own. Code-pattern rules count toward the eight rules of your own, and the browser extension leaves them out.
- **Judgement**, questions the decision model answers from the diff and the documentation:
- **Check the change is focused on one thing.** The decision model is asked whether the pull request does one thing. When it fails, the verdict comment says it mixes unrelated changes and asks the author to split them out, unless the file sets another `onFail`. A pull request based on another open pull request's branch is checked on its own changes, and the comment says so, such as "Stacked on #1573: only this pull request's own changes were checked." On a branch another pull request was merged into, this rule is skipped, and the comment says why, such as "stacked branch, includes #1521".
- **Check docs are updated when behavior changes.** Changing a documentation file passes it. Otherwise the decision model is asked, file by file, whether the docs stay correct, a statement in them is now wrong, or the change adds setup the docs must describe. When it fails, the verdict comment asks the author to update the documentation and names the files, unless the file sets another `onFail`. The default label, when `onFail` is `label`, is `docs: check`.
Expand Down
11 changes: 6 additions & 5 deletions docs/DECISION_MODEL.md
Original file line number Diff line number Diff line change
Expand Up @@ -98,18 +98,19 @@ Two different numbers come out of the answers:
| Requests per pull request | 64 (about 5.8 MB of state, $0.17 at most) | `MAX_JUDGE_REQUESTS` |
| Repository docs | 24,000 bytes, default branch | `REPO_DOCS_MAX_BYTES` |
| Contribution rules | 8,000 bytes, default branch | `REPO_RULES_MAX_BYTES` |
| Patch read per file | 65,536 chars | `MAX_PATCH_INPUT_CHARS` |
| Patch read per pull request | 270,000 chars | `MAX_PATCH_CHARS` |
| Patch the judge reads per file | 65,536 chars | `MAX_PATCH_INPUT_CHARS` |
| Patch the judge reads per pull request | 270,000 chars | `MAX_PATCH_CHARS` |
| Patch the rules that run in code read per pull request | 2,000,000 chars | `MAX_CODE_RULES_PATCH_CHARS` |
| GitHub tool page | 9,000 chars | `PR_FILES_PAGE_CHARS` |

- **Docs.** Docs are read from the default branch in this order: root `README`, then `docs/`, then other root markdown, then nested markdown, each in path order. Reading stops at 24,000 bytes; a doc that crosses the cap is cut and later docs are not seen. The same docs are in every request's state.
- **Contribution rules.** Read under their own 8,000-byte budget, in this order: `AGENTS.md`, `CLAUDE.md`, `CONTRIBUTING` and `PULL_REQUEST_TEMPLATE` at the root, then `CONTRIBUTING` and `PULL_REQUEST_TEMPLATE` in `.github/` (including `PULL_REQUEST_TEMPLATE/`), then the same two in `docs/`. An `AGENTS.md` or `CLAUDE.md` inside a package stays an ordinary doc. Only requests that ask a custom rule carry them and give up room for them; requests that ask only built-in rules are unchanged.
- **Request splitting.** Files fill each request's state in order up to the budget left after metadata and docs. A file too large for one request keeps its leading hunks there for every rule, and focused and docs read its remaining hunks in the next requests, docs asking about each part (any part needing docs fails it); a hunk too large for any request is cut at line ends across several. Each request asks only about the files it holds; whole-PR questions go in the first, and focused asks in each. If the title, body, commits, path list and docs leave less than 8 KB of the budget for files, nothing is asked and the model rules stay unconfirmed ("pull request too large for the decision model (N KB)", the size of that metadata). The same holds when the split needs more than 64 requests ("… (N requests)").
- **Oversized patches.** A patch larger than one tool page is read across pages and joined. Reading stops at 65,536 chars for a file, which is then marked cut. Once the pull request's 270,000-char budget is spent, later files are listed without patches.
- **Unread files.** A file with lines changed but no patch to read (GitHub left it out, the per-PR budget was spent, or not even its first hunk fits a request) is unread, and a rule that would ask about it stays unconfirmed. Treatment differs by rule:
- **Oversized patches.** A patch larger than one tool page is read across pages and joined, and every patch is read whole for the rules that run in code until 2,000,000 chars of the pull request, each pull request in a backlog batch counting its own; lockfiles and other generated files are listed without their patch, so they do not count. The file that passes it partway and every later one are listed without patches. The judge reads 65,536 chars of a file, which is then marked cut. Once the judge's 270,000-char budget is spent, later files are sent to it without patches.
- **Unread files.** A file with lines changed but no patch to read (GitHub left it out, the judge's budget was spent, or not even its first hunk fits a request) is unread, and a rule that would ask about it stays unconfirmed. Treatment differs by rule:
- Docs reads a file too large for one request across requests as focused does. A file whose patch was cut while reading is asked about from the hunks that were read and is unread, so it can fail docs but never pass it. A changed doc file passes docs by rule regardless.
- Scoped custom rules ask about a cut file (its patch cut while reading, or too large for one request) from the hunks that were read and treat it as unread.
- Focused reads every line of every patch it has. Its only unread files are those whose patch it does not have whole: GitHub left it out, the pull request's 270,000-char budget was spent, or the file passed 65,536 chars. Any unread file leaves focused unconfirmed, listing them, unless another slice already fails it.
- Focused reads every line of every patch it has. Its only unread files are those whose patch it does not have whole: GitHub left it out, the judge's 270,000-char budget was spent, or the file passed 65,536 chars. Any unread file leaves focused unconfirmed, listing them, unless another slice already fails it.
- Binary files and pure renames have no lines and are never unread.

## Reused answers
Expand Down
8 changes: 4 additions & 4 deletions packages/triage-workflows/src/actions/facts.ts
Original file line number Diff line number Diff line change
Expand Up @@ -142,11 +142,11 @@ async function readContents(host: HostCall, tool: string, repo: string, sha: str
}

/** Each code-patterns rule's matches, run where the files are read so their text never leaves this step. */
async function patternsOf(host: HostCall, tool: string, repo: string, item: { facts: PrFacts; pack: CheckPack }): Promise<Record<string, PatternsResult>> {
async function patternsOf(host: HostCall, tool: string, repo: string, source: RepoSource, item: { facts: PrFacts; pack: CheckPack }): Promise<Record<string, PatternsResult>> {
const rules = item.pack.custom.filter(isPatternsCheck).filter(customCheckEnabled);
const toRead = filesToRead(item.facts.files);
const toRead = filesToRead(item.facts.files, source);
const contents = Array.isArray(toRead) ? await readContents(host, tool, repo, item.facts.headSha, toRead) : {};
return Object.fromEntries(rules.map((rule) => [rule.id, patternsResult(rule, item.facts.files, contents)]));
return Object.fromEntries(rules.map((rule) => [rule.id, patternsResult(rule, item.facts.files, contents, source)]));
}

/** An open pull request as the host lists it. */
Expand Down Expand Up @@ -369,7 +369,7 @@ async function fetchTargets({ host, stamp }: Host, repo: string, numbers: number
if (!pack.custom.filter(isPatternsCheck).some(customCheckEnabled)) return items;
const matched: RulesItem[] = [];
for (const item of items) {
matched.push("facts" in item ? { ...item, facts: { ...item.facts, patterns: await patternsOf(host, tool("read_file"), repo, item) } } : item);
matched.push("facts" in item ? { ...item, facts: { ...item.facts, patterns: await patternsOf(host, tool("read_file"), repo, stamp.repoSource, item) } } : item);
}
return matched;
}
Expand Down
Loading
Loading