Repository navigation
refactor(review): build the incremental scope from the PR's diff, not a check #9267
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 3 commits
c50f173
a4339fe
0806cbc
bba8deb
1a55008
1e592dc
59207d8
356aecf
ceccd55
62572c8
2dde9e6
cef5952
b66bded
8c5d94b
c25efbe
8ae07e3
45bf8fc
9d53584
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -1332,11 +1332,13 @@ export function buildRoleBrief( | |
| `\`${wt}\`. Do not \`cd\` elsewhere and do not build the user's main checkout.`, | ||
| ); | ||
| } | ||
| // On a delta-scoped incremental round the probe's range must match the | ||
| // round's scope: test-efficacy recomputes its own diff as base..HEAD, and | ||
| // handed the merge base it would reverse hunks and delete mutants from | ||
| // commits an earlier round already reviewed — spending the probe budget | ||
| // out of scope and reporting survivors this round's diff never contains. | ||
| // On a narrowed incremental round the probe's range must cover the | ||
| // published scope: test-efficacy recomputes its own diff as base..HEAD. | ||
| // The published hunks are hunks of `diffBase..head` — the merge-base | ||
| // range the producer assembled them from — so that range covers every | ||
| // one of them and never a byte the PR's diff does not display; the | ||
|
Comment on lines
+1546
to
+1548
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Deferred to the next round. The soundness half of this finding is already documented in code (the comment at the weld site states the superset argument), and the budget cost is real — but the two available fixes differ sharply in size. Restoring the trade-off warning is a comment; constraining the probe to the published scope needs a new seam (per-file scope ranges from the fetch report into test-efficacy's mutant/hunk selection) with its own tests. This round's batch budget went to the Critical (R4-1) and the battery-power scenarios first; the probe-scope fix is queued for the next round, where the seam can be tested on its own. Until then the spend is not silent: 中文说明推迟至下一轮。本发现中「可靠性」的一半已在代码中记录在案(焊接处的注释已陈述超集论证),预算代价也确实存在——但两种可行修复的体量相差悬殊。把权衡警告写回原处只是一条注释;而把探针约束到发布范围需要一条新的数据接缝(从 fetch 报告把逐文件范围传给 test-efficacy 的 mutant/hunk 选择),并需要配套的测试。本轮的批处理预算优先用于 Critical(R4-1)与各测试组威力场景;探针范围的修复排入下一轮,届时该接缝可以单独获得测试。在此之前,这部分消耗并非无声无息:每一份 efficacy 报告都会通过 |
||
| // anchor range, by contrast, can carry hunks an undo round netted out | ||
| // of the PR's diff, which no comment can anchor on. | ||
| const inc = report.incremental as | ||
| | { effective?: unknown; upToDate?: unknown; diffBase?: unknown } | ||
| | undefined; | ||
|
|
||
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Deferred to the follow-up queue (verified real, fix out of this PR's footprint). The claim is confirmed at this commit by code inspection:
fetch-pr.ts's publish arm setsanchor.incremental.diffBase = mergeBaseSha, so the incremental arm inagent-prompt.ts(shaOrNull(inc.diffBase) ?? shaOrNull(report.mergeBaseSha)) always resolves to the merge base, and on every effective incremental round the test-efficacy probe recomputesmergeBase..HEAD— spending mutant/hunk selection over the PR's entire range rather than the round's scope. Both fixes discussed in this thread (feeding test-efficacy the published diff/plan, or welding the probe base to the anchor range with hunk-level filtering against the published plan) require changingtest-efficacy.ts's own range recomputation, a file this PR never touches, and they differ sharply in size — so the choice belongs to a follow-up rather than this PR's remaining rounds. The thread stays open and the finding is recorded in the deferred-findings queue so it survives merge.中文说明
推迟到后续跟进队列(已核实属实,修复超出本 PR 的 footprint)。通过在本提交上读码确认该论断:
fetch-pr.ts的发布分支把anchor.incremental.diffBase设为mergeBaseSha,因此agent-prompt.ts的增量分支(shaOrNull(inc.diffBase) ?? shaOrNull(report.mergeBaseSha))总是落到 merge base,每个有效增量轮中 test-efficacy 探针都重算mergeBase..HEAD——把突变体/hunk 选择预算花在整个 PR 区间上,而不是本轮范围。线程中讨论的两种修复(把已发布的 diff/plan 交给 test-efficacy,或把探针 base 焊到锚点区间并按已发布 plan 做 hunk 级过滤)都需要修改test-efficacy.ts自身的区间重算——该文件不在本 PR 改动范围内——且两者规模差异很大,因此取舍应由后续 PR 决定,不属于本 PR 的剩余轮次。线程保持打开,发现已记入 deferred-findings 队列,合并后不会丢失。