Skip to content

Commit fe93915

Browse files
committed
fix(triage): judge the first 64 requests of a diff that needs more and leave the rest unread
1 parent 82e6866 commit fe93915

4 files changed

Lines changed: 52 additions & 23 deletions

File tree

‎docs/ARCHITECTURE.md‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -557,8 +557,9 @@ label off that head, so it never stands on a head not yet triaged.
557557
without patches. The judge reads the same patches (`judgedFiles` in
558558
`logic/quality.ts`) across up to 64 first-round requests, plus the verdict
559559
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
560+
the first round sends the first 64, and the files only later requests hold
561+
are unread, so the judged rules that ask about them are Unsure, naming them.
562+
A match in any checked file fails the rule. Without one, a
562563
file that adds code but has no patch, because the host sent none (GitHub
563564
omits it for a very large or binary file) or the pull request passed that
564565
ceiling, or whose content was not read, leaves the rule unconfirmed, naming

‎docs/DECISION_MODEL.md‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ Questions point into the state by path, such as `` `pullRequest.files[3].hunks[0
5151
- A doc file is `*.md` or `*.mdx` outside `vendor/`, `node_modules/`, `dist/` and `build/`, legal notices excluded.
5252
- A test file is under a `test/`, `tests/`, `Tests/` or `__tests__/` directory (so Rust's `tests/`, Maven's `src/test/` and Swift's `Tests/` count) or a C# `*.Tests/` project such as `Acme.UnitTests/`, a Ruby file under `spec/`, or named by its language's convention: `*.test.*` or `*.spec.*` in JS or TS, `test_*.py` or `*_test.py`, `*_test.go`, `test_*.rb`, `*_test.rb` or `*_spec.rb`, `*_test.exs`, `*_test.dart`, `*Test` or `*Tests` in Java, Kotlin and C#, and `*Test`, `*Spec` or `*Suite` in Scala, the last two outside `src/main/`. It is not asked about.
5353
- Otherwise each file gets one Choice, "How does the change in `pullRequest.files[N]` leave `repository.docs`?", with options in this order: `unaffected` (the docs stay correct and complete), `now_wrong` (a statement the docs make, such as a name, table, route, file, setting or behaviour, is contradicted because the change renames, removes or changes it) and `now_incomplete` (the change adds setup, configuration, permissions or commands the docs must describe). P(needs docs) is 1 − P(`unaffected`), on the Noul band: a file at 0.7 or above needs docs and fails the rule, one at 0.3 or below does not, and anything between leaves the rule unconfirmed. At most 3 files are named.
54-
- **Duplicates open pull request**: the facts step shortlists up to five open pull requests opened earlier on the same base branch that share a changed path (lockfiles left out) or two title words of four or more letters with this one, most shared paths first. With none, it passes without a question. Otherwise one request of its own holds this pull request's metadata (no diff) and the shortlist as `repository.openPullRequests` (number, title and up to 50 changed paths each), and asks one Score per shortlisted pull request, question id `duplicates-open-pull-request-NNN`: "How does this pull request relate to `repository.openPullRequests[N]`, another open pull request, by both titles, this pull request's body and both lists of changed paths?", with levels `same change`, `overlaps` and `unrelated`, each described in the question. The pull request with the highest P(`same change`) decides it on the unsure band: 0.7 or above fails it and names that pull request, 0.3 or below passes it, and anything between is unconfirmed. Its default `onFail` is `ask`, so a failure makes the verdict Unsure. The shortlist sits under `repository`, which the instructed question covers, since other pull requests' titles are untrusted too. When its request would take the pull request past 64 requests, it is not asked and the rule is unconfirmed. The shortlist is measured without a model (`bun run eval:duplicates`, 8 cases: 8 of 8 put the true duplicate on it and keep lockfile-only and common-word overlaps off). The judge's answer is unmeasured: no eval case asks it, so how often a failure names a real duplicate is unknown.
54+
- **Duplicates open pull request**: the facts step shortlists up to five open pull requests opened earlier on the same base branch that share a changed path (lockfiles left out) or two title words of four or more letters with this one, most shared paths first. With none, it passes without a question. Otherwise one request of its own holds this pull request's metadata (no diff) and the shortlist as `repository.openPullRequests` (number, title and up to 50 changed paths each), and asks one Score per shortlisted pull request, question id `duplicates-open-pull-request-NNN`: "How does this pull request relate to `repository.openPullRequests[N]`, another open pull request, by both titles, this pull request's body and both lists of changed paths?", with levels `same change`, `overlaps` and `unrelated`, each described in the question. The pull request with the highest P(`same change`) decides it on the unsure band: 0.7 or above fails it and names that pull request, 0.3 or below passes it, and anything between is unconfirmed. Its default `onFail` is `ask`, so a failure makes the verdict Unsure. The shortlist sits under `repository`, which the instructed question covers, since other pull requests' titles are untrusted too. Its request always has a slot: when the split needs more, the files give one up. The shortlist is measured without a model (`bun run eval:duplicates`, 8 cases: 8 of 8 put the true duplicate on it and keep lockfile-only and common-word overlaps off). The judge's answer is unmeasured: no eval case asks it, so how often a failure names a real duplicate is unknown.
5555
- **Custom model rules** (`packages/triage-contracts/src/check-pack.ts`):
5656
- `is-true` with scope `pull-request` (the default): one Noul, `<name>: <claim>`.
5757
- `is-true` with scope `file` or `hunk`, optionally limited by `when` globs: one Noul per file or hunk in scope, "Does the change in … meet this rule: <claim>?". At most 3 items are named on failure.
@@ -105,12 +105,12 @@ Two different numbers come out of the answers:
105105

106106
- **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.
107107
- **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.
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)".
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). When the split needs more than 64 requests (63 when duplicates has a request), the first 64 (or 63) are sent and the files only later requests hold are unread.
109109
- **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:
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, not even its first hunk fits a request, or only requests past the 64th, or the 63rd when duplicates has a request, hold it) is unread, and a rule that would ask about it stays unconfirmed. Treatment differs by rule:
111111
- 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.
112112
- 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.
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.
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, the file passed 65,536 chars, or part of it was only in requests past the 64th (the 63rd when duplicates has a request). Any unread file leaves focused unconfirmed, listing them, unless another slice already fails it.
114114
- Binary files and pure renames have no lines and are never unread.
115115

116116
## Reused answers
@@ -128,7 +128,7 @@ Estimates at TypeSafe's direct price, about 3.3 bytes per token (32k tokens ≈
128128

129129
- 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.
130130
- One full request: at most 64k tokens, about $0.0027.
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.
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 has its later files unread, so the rules that ask about them are Unsure, naming them. 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, and files past the 64th request (the 63rd when duplicates has a request) are unread.
132132
- 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.
133133
- 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.
134134

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

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { expect, test } from "bun:test";
22
import type { PrFacts, PrFileFacts } from "./checks.js";
33
import { patchHunks } from "./hunks.js";
44
import { NO_FACTS } from "./item.js";
5-
import { focusedPartRequests, MAX_SYSTEM_ONE_QUESTIONS, MAX_SYSTEM_ONE_STATE_BYTES, prepareQualityEvaluation, type JudgeRequest } from "./quality.js";
5+
import { focusedPartRequests, MAX_CODE_RULES_PATCH_CHARS, MAX_SYSTEM_ONE_QUESTIONS, MAX_SYSTEM_ONE_STATE_BYTES, prepareQualityEvaluation, type JudgeRequest } from "./quality.js";
66

77
const SOURCES = { quality: [{ id: "focused" as const, group: "pull-request" as const }], custom: [], rules: [] };
88

@@ -130,23 +130,31 @@ test("quality preflight lists only the changed paths that fit focused's share, w
130130
expect(Object.values(directories!).reduce((total, row) => total + row.files, 0)).toBe(100);
131131
});
132132

133-
test("quality preflight refuses metadata that leaves too little room for files, and more requests than the cap, instead of asking thousands", () => {
133+
test("quality preflight refuses metadata that leaves too little room for files instead of asking thousands", () => {
134134
const patch = compactHunks(1);
135135
const subject = prepareQualityEvaluation({ ...facts([{ path: "src/a.ts", patch }]), commits: ["x".repeat(90_000)] }, SOURCES);
136136
expect(subject.requests[0]!.state.pullRequest.commits).toEqual(["x".repeat(200)]);
137137
const crowded = prepareQualityEvaluation({ ...facts([{ path: "src/a.ts", patch }]), docs: { "README.md": "d".repeat(85_000) } }, SOURCES);
138138
expect(crowded).toMatchObject({ requests: [], judgeError: expect.stringMatching(/^pull request too large for the decision model \(\d+ KB\)$/) });
139-
const wide = prepareQualityEvaluation({ ...facts(Array.from({ length: 40 }, (_, index) => ({ path: `src/wide${index}.ts`, patch: widePatch() }))), docs: { "README.md": "d".repeat(78_000) } }, SOURCES);
140-
expect(wide).toMatchObject({ requests: [], judgeError: expect.stringMatching(/^pull request too large for the decision model \(\d+ requests\)$/) });
141139
});
142140

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)" });
141+
test("quality preflight judges a diff under the facts ceiling that needs more than 64 requests, reading the first 63 plus the duplicates request and naming the rest unread", () => {
142+
const files = Array.from({ length: 71 }, (_, index) => ({ path: `src/${"deeply/nested/".repeat(6)}module${index}.ts`, patch: `@@ -0,0 +1 @@\n+// ${"x".repeat(27_980)}` }));
143+
expect(files.reduce((chars, file) => chars + file.patch.length, 0)).toBeLessThan(MAX_CODE_RULES_PATCH_CHARS);
144+
const sources = { ...SOURCES, quality: [...SOURCES.quality, { id: "docs" as const, group: "pull-request" as const }, { id: "duplicates-open-pull-request" as const, group: "pull-request" as const }] };
145+
const openPulls = { overlapping: [{ number: 7, title: "Move modules", paths: [files[0]!.path] }] };
146+
const { requests, plan, focused, judgeError } = prepareQualityEvaluation({ ...facts(files), paths: files.map((file) => file.path), body: "b".repeat(8_000), docs: { "README.md": "d".repeat(24_000) }, openPulls }, sources);
147+
expect(judgeError).toBeUndefined();
148+
expect(requests).toHaveLength(64);
149+
expect(requests.at(-1)!.state).toMatchObject({ repository: { openPullRequests: openPulls.overlapping } });
150+
expect(plan["duplicates-open-pull-request"]).toEqual({ changed: [], asked: { "duplicates-open-pull-request-001": "#7" }, unread: [] });
151+
const read = requests.flatMap((request) => request.state.pullRequest.files.map((file) => file.path));
152+
const unread = files.map((file) => file.path).filter((path) => !read.includes(path));
153+
expect(unread).toHaveLength(8);
154+
expect(focused!.unread).toEqual(unread);
155+
expect(Object.keys(focused!.slices)).toHaveLength(63);
156+
expect(plan.docs!.unread).toEqual(unread);
157+
expect(Object.values(plan.docs!.asked)).toEqual(read);
150158
});
151159

152160
test("quality preflight leaves a cut patch unread for focused even when its first hunk is read in pieces", () => {

0 commit comments

Comments
 (0)