Skip to content

Commit 37ada0c

Browse files
committed
fix(triage): let the judge read every patch the facts step read
The judge read patches only up to 270,000 chars, three requests' worth, so on a larger pull request the files past it reached the judge without lines and focused and docs stayed unconfirmed. The facts step's 2,000,000-char ceiling is below what 64 requests hold, so the judge keeps no patch budget of its own: it reads every patch the facts step read, each cut at 65,536 chars, across up to 64 first-round requests, now its only bound and pinned by a test. DECISION_MODEL.md states the worst case, 64 first-round requests, the verdict request and at most 30 focused re-sends, about $0.26, and says a diff needing more than 64 in the first round leaves every judged rule Unsure as too large for the decision model.
1 parent b3feb80 commit 37ada0c

6 files changed

Lines changed: 51 additions & 65 deletions

File tree

‎docs/ARCHITECTURE.md‎

Lines changed: 23 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -552,26 +552,29 @@ label off that head, so it never stands on a head not yet triaged.
552552
pages as it takes, until it has read 2,000,000 characters of the pull
553553
request (`MAX_CODE_RULES_PATCH_CHARS`), each pull request in a batch
554554
counting its own, since one sidecar holds every deployment's step outputs;
555-
lockfiles and other generated files are listed without their patch, which
556-
no rule reads. The file that passes it partway and every later one are
557-
listed without patches. The judge's view of the diff has its own, smaller
558-
budget (`judgedFiles` in `logic/quality.ts`). A match in any checked file
559-
fails the rule. Without one, a file that adds code but has no patch,
560-
because the host sent none (GitHub omits it for a very large or binary
561-
file) or the pull request passed that ceiling, or whose content was not
562-
read, leaves the rule unconfirmed, naming the files and why; a file GitHub
563-
counts no added lines in, such as a rename or a mode change, has nothing to
564-
check. A rule ast-grep refuses leaves the rule unconfirmed. `vendor/`,
565-
`dist/`, `build/` and `generated/` at the repository root and
566-
`node_modules/` anywhere are skipped and listed as skipped in the evidence,
567-
and a `?? 0` inside arithmetic is a counter's real default. With such a
568-
rule on, the facts step reads each file whose patch adds lines at the head
569-
through the host's `read_file` (`whole`), ten at a time and at most 300 per
570-
pull request (more leaves the rule unconfirmed without reading any), runs
571-
the rules there and outputs only the matches. Bitbucket Data Center lists
572-
no patch, so there the rule is unconfirmed whenever an in-scope TypeScript
573-
or JavaScript file is added, edited, moved or copied. The extension leaves
574-
code-pattern rules out and swaps the native module for a stub.
555+
lockfiles and other generated files are listed without their patch, which no
556+
rule reads. The file that passes it partway and every later one are listed
557+
without patches. The judge reads the same patches (`judgedFiles` in
558+
`logic/quality.ts`) across up to 64 first-round requests, plus the verdict
559+
request and at most 30 focused re-sends; a diff that needs more than 64 in
560+
the first round leaves every judged rule Unsure as too large for the
561+
decision model. A match in any checked file fails the rule. Without one, a
562+
file that adds code but has no patch, because the host sent none (GitHub
563+
omits it for a very large or binary file) or the pull request passed that
564+
ceiling, or whose content was not read, leaves the rule unconfirmed, naming
565+
the files and why; a file GitHub counts no added lines in, such as a rename
566+
or a mode change, has nothing to check. A rule ast-grep refuses leaves the
567+
rule unconfirmed. `vendor/`, `dist/`, `build/` and `generated/` at the
568+
repository root and `node_modules/` anywhere are skipped and listed as
569+
skipped in the evidence, and a `?? 0` inside arithmetic is a counter's real
570+
default. With such a rule on, the facts step reads each file whose patch
571+
adds lines at the head through the host's `read_file` (`whole`), ten at a
572+
time and at most 300 per pull request (more leaves the rule unconfirmed
573+
without reading any), runs the rules there and outputs only the matches.
574+
Bitbucket Data Center lists no patch, so there the rule is unconfirmed
575+
whenever an in-scope TypeScript or JavaScript file is added, edited, moved
576+
or copied. The extension leaves code-pattern rules out and swaps the native
577+
module for a stub.
575578
3. **route**: skips the judge when no question needs asking.
576579
4. **judge**: one System One call per request of questions. Each first-round request and the verdict request also asks
577580
whether the pull request's text, diff or docs try to direct the judge; a

‎docs/DECISION_MODEL.md‎

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -95,23 +95,22 @@ Two different numbers come out of the answers:
9595
| State per request | 90,000 bytes (32k tokens is about 105 KB of diff) | `MAX_SYSTEM_ONE_STATE_BYTES` |
9696
| Questions per request | 256 (no documented cap; 300 were answered) | `MAX_SYSTEM_ONE_QUESTIONS` |
9797
| Room for files per request | at least 8 KB | `MIN_FILE_ROOM` |
98-
| Requests per pull request | 64 (about 5.8 MB of state, $0.17 at most) | `MAX_JUDGE_REQUESTS` |
98+
| First-round requests per pull request | 64 (about 5.8 MB of state) | `MAX_JUDGE_REQUESTS` |
9999
| Repository docs | 24,000 bytes, default branch | `REPO_DOCS_MAX_BYTES` |
100100
| Contribution rules | 8,000 bytes, default branch | `REPO_RULES_MAX_BYTES` |
101101
| Patch the judge reads per file | 65,536 chars | `MAX_PATCH_INPUT_CHARS` |
102-
| Patch the judge reads per pull request | 270,000 chars | `MAX_PATCH_CHARS` |
103-
| Patch the rules that run in code read per pull request | 2,000,000 chars | `MAX_CODE_RULES_PATCH_CHARS` |
102+
| Patch the facts step reads per pull request, for the rules that run in code and the judge | 2,000,000 chars | `MAX_CODE_RULES_PATCH_CHARS` |
104103
| Patch text in one facts step's output | 2,000,000 chars: one pull request per step, as the hub records backlog and catch-up mails per pull request and each manager reads its own | `MAX_CODE_RULES_PATCH_CHARS` × 1 |
105104
| GitHub tool page | 9,000 chars | `PR_FILES_PAGE_CHARS` |
106105

107106
- **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.
108107
- **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.
109-
- **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)").
110-
- **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.
111-
- **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:
108+
- **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: no request is sent and every judged rule is Unsure with "pull request too large for the decision model (N requests)".
109+
- **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 every patch the facts step read, up to 65,536 chars of a file, which is then marked cut, across as many first-round requests as it takes up to 64.
110+
- **Unread files.** A file with lines changed but no patch to read (GitHub left it out, the facts step passed its 2,000,000-char ceiling, 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:
112111
- 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.
113112
- 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.
114-
- 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.
113+
- 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 facts step passed its 2,000,000-char ceiling, or the file passed 65,536 chars. Any unread file leaves focused unconfirmed, listing them, unless another slice already fails it.
115114
- Binary files and pure renames have no lines and are never unread.
116115

117116
## Reused answers
@@ -129,9 +128,9 @@ Estimates at TypeSafe's direct price, about 3.3 bytes per token (32k tokens ≈
129128

130129
- A small pull request: one request. With 24 KB of docs and a few KB of diff that is about 9k tokens, about $0.0004.
131130
- One full request: at most 64k tokens, about $0.0027.
132-
- The largest pull request Triage reads: the 270,000-char patch budget fills about four to five requests when the docs take their full 24 KB, at most about $0.015. Hunk-scoped custom rules add one question per hunk, uncapped, so wide diffs can need more requests.
131+
- The largest pull request Triage judges: 64 first-round requests, the verdict request and, when focused fails, at most 30 re-sent requests (below), so at most 95 requests, about $0.26 at $0.0027 per full request. Files go into requests whole, so a diff fills more requests than its size alone suggests; one that needs more than 64 in the first round leaves every judged rule Unsure with "pull request too large for the decision model (N requests)". corbits-triage#688, 141 files and about 620,000 chars of patch, takes 9 requests, at most about $0.025. Hunk-scoped custom rules add one question per hunk, uncapped, so wide diffs can need more requests.
133132
- Focused asks one question in each request that holds files. It adds requests only for slices no other rule needs: the rest of a file too large for one request, or files no other rule asks about. On the judge eval the largest pull requests (#46, #119, #157, #158) take 2 calls each. With 24 KB of docs and small patches, a pull request of 200 files takes 2 requests, 500 files 4, 1,000 files 8 and 3,000 files 24, under 66 KB of state each.
134-
- A failed focused rule sends each failing slice's request again with its file questions: one extra request per failing slice, the same state re-sent whole. On the judge eval's mixes that is one or two extra requests of 34–83 KB each, about $0.0004–0.0011 apiece.
133+
- A failed focused rule sends each failing slice's request again with its file questions: one extra request per failing slice, the same state re-sent whole, until 30 files are named. On the judge eval's mixes that is one or two extra requests of 34–83 KB each, about $0.0004–0.0011 apiece.
135134

136135
## Measured accuracy
137136

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

Lines changed: 1 addition & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import type { PrFileFacts } from "./checks.ts";
77
import { addFilesPage, assembleItem, buildFacts, FIRST_FILES_PAGE, type FilesCursor, type FilesPage, type ListedFiles } from "./facts.ts";
88
import { NO_FACTS } from "./item.ts";
99
import { patternsResult } from "./patterns.ts";
10-
import { MAX_CODE_RULES_PATCH_CHARS, MAX_SYSTEM_ONE_STATE_BYTES, prepareQualityEvaluation } from "./quality.ts";
10+
import { MAX_CODE_RULES_PATCH_CHARS } from "./quality.ts";
1111

1212
test("buildFacts carries the author's trust tier", () => {
1313
const facts = buildFacts("acme/widgets", 1, { author: "alice", authorAssociation: "MEMBER", branch: "a" }, [], [], [], DEFAULT_REPO_POLICY);
@@ -104,28 +104,6 @@ function readPages(hostFiles: PrFileFacts[], first: FilesCursor = FIRST_FILES_PA
104104
return listed.files;
105105
}
106106

107-
test("a diff past the judge's budget is read whole for the pattern rules while the judge reads a bounded part", () => {
108-
const filler = Array.from({ length: 200 }, (_, line) => `const filler${line} = "${"x".repeat(30)}";`);
109-
const fallback = "const late = value ?? 0;";
110-
const paths = Array.from({ length: 40 }, (_, index) => `src/file${String(index).padStart(2, "0")}.ts`);
111-
const lines = (index: number) => (index === paths.length - 1 ? [...filler, fallback] : filler);
112-
const patch = (index: number) => [`@@ -0,0 +1,${lines(index).length} @@`, ...lines(index).map((line) => `+${line}`)].join("\n");
113-
const hostFiles: PrFileFacts[] = paths.map((path, index) => ({ path, status: "added", additions: lines(index).length, deletions: 0, patch: patch(index) }));
114-
const listed = readPages(hostFiles);
115-
expect(listed).toEqual(hostFiles);
116-
expect(hostFiles.reduce((chars, file) => chars + file.patch!.length, 0)).toBeGreaterThan(3 * MAX_SYSTEM_ONE_STATE_BYTES);
117-
118-
const pack = readCheckPack(readPolicyFile("rules:\n - id: conventions\n check: code-patterns\n text: Conventions.\n reason: Review time.\n evidence: [maintainer-stated]\n presets: [convention]\n", "acme/widgets"));
119-
const contents = Object.fromEntries(paths.map((path, index) => [path, `${lines(index).join("\n")}\n`]));
120-
expect(patternsResult(pack.custom.find(isPatternsCheck)!, listed, contents, "github")).toEqual({ matches: [`${paths.at(-1)}:201 Invented fallback \`?? 0\`; handle the missing value instead.`], skipped: [], checked: paths.length });
121-
122-
const focused = { quality: [{ id: "focused" as const, group: "pull-request" as const }], custom: [], rules: [] };
123-
const { requests, focused: plan } = prepareQualityEvaluation({ ...NO_FACTS, repo: "acme/widgets", number: 8, title: "Add files", files: listed, paths }, focused);
124-
const judged = requests.flatMap((request) => request.state.pullRequest.files.flatMap((file) => file.hunks)).join("").length;
125-
expect(judged).toBeLessThanOrEqual(3 * MAX_SYSTEM_ONE_STATE_BYTES + 65_536);
126-
expect(plan!.unread).toContain(paths.at(-1)!);
127-
});
128-
129107
describe("a diff past the code rules' ceiling", () => {
130108
const pack = readCheckPack(readPolicyFile("rules:\n - id: conventions\n check: code-patterns\n text: Conventions.\n reason: Review time.\n evidence: [maintainer-stated]\n presets: [convention]\n", "acme/widgets"));
131109

‎packages/triage-workflows/src/logic/quality.test.ts‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -140,6 +140,15 @@ test("quality preflight refuses metadata that leaves too little room for files,
140140
expect(wide).toMatchObject({ requests: [], judgeError: expect.stringMatching(/^pull request too large for the decision model \(\d+ requests\)$/) });
141141
});
142142

143+
test("quality preflight sends 64 requests and refuses the 65th, its only bound on the diff it reads", () => {
144+
const halfRequest = (index: number) => ({ path: `src/half${index}.ts`, patch: `@@ -0,0 +1 @@\n+// ${"x".repeat(50_000)}` });
145+
const atCap = prepareQualityEvaluation(facts(Array.from({ length: 64 }, (_, index) => halfRequest(index))), SOURCES);
146+
expect(atCap.judgeError).toBeUndefined();
147+
expect(atCap.requests).toHaveLength(64);
148+
const pastCap = prepareQualityEvaluation(facts(Array.from({ length: 65 }, (_, index) => halfRequest(index))), SOURCES);
149+
expect(pastCap).toMatchObject({ requests: [], judgeError: "pull request too large for the decision model (65 requests)" });
150+
});
151+
143152
test("quality preflight leaves a cut patch unread for focused even when its first hunk is read in pieces", () => {
144153
const patch = ["@@ -0,0 +1,70 @@", ...Array.from({ length: 70 }, () => `+// ${"設".repeat(1_000)}`)].join("\n");
145154
const { focused } = prepareQualityEvaluation(facts([{ path: "src/big.ts", additions: 70, deletions: 0, patch }]), SOURCES);

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

Lines changed: 4 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -178,26 +178,22 @@ export const MAX_SYSTEM_ONE_STATE_BYTES = 90_000;
178178
export const MAX_SYSTEM_ONE_QUESTIONS = 256;
179179
// Below this much room for files a hunk is cut into so many pieces that each costs a request of mostly metadata.
180180
const MIN_FILE_ROOM = 8 * 1024;
181-
// About 5.8 MB of state; the 270,000-char patch budget fills about five requests, 3,000 small files 24.
181+
// About 5.8 MB of state; 3,000 small files take 24.
182182
const MAX_JUDGE_REQUESTS = 64;
183183
export const QUALITY_EVALUATION_LIMIT_ERROR = "pull request too large for the decision model";
184184

185-
// The judge reads patches up to three requests' worth; later files are sent without theirs.
186-
const MAX_PATCH_CHARS = 3 * MAX_SYSTEM_ONE_STATE_BYTES;
187-
// The rules that run in code read this much patch text per pull request, apart from the judge's budget; one sidecar
185+
// The facts step reads this much patch text per pull request, for the rules that run in code and the judge; one sidecar
188186
// holds every deployment's step outputs, so past it the pull request's later files are listed without their patches.
189187
export const MAX_CODE_RULES_PATCH_CHARS = 2_000_000;
190188

191189
/** A changed file as the judge reads it: `patchTruncated` when its patch was cut at `MAX_PATCH_INPUT_CHARS`. */
192190
type JudgedFile = PrFileFacts & { patchTruncated?: true };
193191

194-
/** Lockfiles say nothing the judge decides on, so it never reads them; the rest share the judge's patch budget. */
192+
/** Lockfiles say nothing the judge decides on, so it never reads them; the rest it reads as the facts step read them. */
195193
function judgedFiles(facts: PrFacts): JudgedFile[] {
196-
let chars = 0;
197194
return facts.files.filter((file) => !isGeneratedPath(file.path)).map(function judged({ patch, ...file }): JudgedFile {
198-
if (patch === undefined || chars >= MAX_PATCH_CHARS) return file;
195+
if (patch === undefined) return file;
199196
const read = patch.slice(0, MAX_PATCH_INPUT_CHARS);
200-
chars += read.length;
201197
return { ...file, patch: read, ...(read.length < patch.length && { patchTruncated: true as const }) };
202198
});
203199
}

0 commit comments

Comments
 (0)