Skip to content

Commit 17b6ca3

Browse files
authored
test(coverage): rename-only hunks owe no changed-line coverage (#2248)
* test(coverage): rename-only hunks owe no changed-line coverage Pass --find-renames=90% to the changed-line diff so rename detection no longer depends on the host diff.renames setting: a 100%-similarity move contributes no changed lines and an edited move contributes only the hunks that differ from its source. Threshold unchanged. * docs(agents): pure moves carry their tests unchanged Drops the stale src/daemon/handlers/session.ts over-budget bullet (242 lines on main) to stay under the AGENTS.md byte budget. * style: format coverage-changed run.ts * docs(agents): restore the session.ts over-budget rule
1 parent 172ee14 commit 17b6ca3

4 files changed

Lines changed: 98 additions & 11 deletions

File tree

AGENTS.md

Lines changed: 7 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,8 +18,8 @@ task procedures only when needed:
1818
| Writing issues or PRDs, and triage labels | `docs/agents/issue-tracker.md`, `docs/agents/triage-labels.md` |
1919
| Web backend setup or diagnostics | `docs/agents/web-backend.md` |
2020

21-
Versioned CLI help is the source of truth for command behavior. Start workflow planning with
22-
`agent-device help workflow`, then use the relevant topic help.
21+
Versioned CLI help is the source of truth for command behavior. Start with `agent-device help
22+
workflow`, then the relevant topic help.
2323

2424
## Incident-derived principles
2525

@@ -91,15 +91,16 @@ cross-language rules change through golden tables under `contracts/fixtures/`.
9191
- Implementation files target at most 300 lines. Extract before adding behavior past 500 lines;
9292
files past 1,000 lines are architecture debt unless generated or fixture data.
9393
- Tests mirror source topology one-to-one. Split a source module and its test together; do not add to
94-
the legacy `interaction.test.ts` or platform `index.test.ts` aggregations.
94+
the legacy `interaction.test.ts` or platform `index.test.ts` aggregations. Pure moves carry their
95+
tests unchanged; rename-only hunks owe no new coverage.
9596
- Shared fixtures are named exports in a sibling fixture module, not repeated inline literals.
9697
- `src/daemon/handlers/session.ts` is already over budget; extract the relevant platform-specific
9798
concept before adding behavior.
9899

99100
## Toolchain and worktree traps
100101

101-
- Use `pnpm`; never add `package-lock.json`. OXC owns lint and format. Run `pnpm format` for the
102-
repository, not a path-scoped formatter invocation.
102+
- Use `pnpm`; never add `package-lock.json`. OXC owns lint and format. Run `pnpm format`
103+
repository-wide, not path-scoped.
103104
- A fresh worktree requires `pnpm install --frozen-lockfile && pnpm build`. Until then package and
104105
optional-peer resolution may point at another checkout and produce false failures.
105106
- Source-checkout daemon state is worktree-scoped, but devices are not. Use `pnpm daemon:state-dir`
@@ -155,5 +156,5 @@ contents. Keep a sentence in this file only when no gate, lint rule, versioned h
155156
decision-site comment can own it. `CONTEXT.md` is glossary-only: no implementation paths,
156157
architecture decisions, migration state, or workflows.
157158

158-
Behavior changes update their owning help/metadata and user docs when relevant. In the final summary,
159+
Behavior changes update their owning help/metadata and user docs. In the final summary,
159160
state whether docs or skills changed and why.

scripts/coverage-changed/model.test.ts

Lines changed: 71 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -239,3 +239,74 @@ test('changed line suppressed by an ignore directive is tallied, not gated', ()
239239
test('threshold constant is the single source of truth', () => {
240240
assert.equal(CHANGED_LINE_COVERAGE_THRESHOLD, 70);
241241
});
242+
243+
// Shape of `git diff --unified=0 --find-renames=90%` for a move PR: a pure
244+
// move carries no hunks, an edited move carries only the hunks that differ from
245+
// its source, and an ordinary edit is unaffected.
246+
const MOVE_PR_DIFF = [
247+
'diff --git a/src/mod.ts b/src/mod.ts',
248+
'index 57ee600..9fe64ef 100644',
249+
'--- a/src/mod.ts',
250+
'+++ b/src/mod.ts',
251+
'@@ -2 +2,2 @@ export const m1 = 1;',
252+
'-export const m2 = 2;',
253+
'+export const m2 = 22;',
254+
'+export const m3 = 3;',
255+
'diff --git a/src/edited.ts b/src/moved-edited.ts',
256+
'similarity index 90%',
257+
'rename from src/edited.ts',
258+
'rename to src/moved-edited.ts',
259+
'index a3d7378..fe85380 100644',
260+
'--- a/src/edited.ts',
261+
'+++ b/src/moved-edited.ts',
262+
'@@ -3 +3 @@ export const b2 = 2;',
263+
'-export const b3 = 3;',
264+
'+export const b3 = 33;',
265+
'@@ -7 +7 @@ export const b6 = 6;',
266+
'-export const b7 = 7;',
267+
'+export const b7 = 77;',
268+
'@@ -9 +9 @@ export const b8 = 8;',
269+
'-export const b9 = 9;',
270+
'+export const b9 = 99;',
271+
'diff --git a/src/pure.ts b/src/moved-pure.ts',
272+
'similarity index 100%',
273+
'rename from src/pure.ts',
274+
'rename to src/moved-pure.ts',
275+
].join('\n');
276+
277+
function uncoveredRecord(file: string, lineCount: number): string {
278+
const da = Array.from({ length: lineCount }, (_, i) => `DA:${i + 1},0`);
279+
return [`SF:${file}`, ...da, 'end_of_record'].join('\n');
280+
}
281+
282+
test('rename-only hunks owe no changed-line coverage; edited moves owe their edits', () => {
283+
const diffs = parseUnifiedDiff(MOVE_PR_DIFF);
284+
assert.deepEqual(
285+
diffs.map((d) => [d.path, d.added]),
286+
[
287+
['src/mod.ts', [2, 3]],
288+
['src/moved-edited.ts', [3, 7, 9]],
289+
['src/moved-pure.ts', []],
290+
],
291+
);
292+
const result = computeChangedCoverage({
293+
diffs,
294+
coverage: parseLcov(
295+
[
296+
uncoveredRecord('src/mod.ts', 3),
297+
uncoveredRecord('src/moved-edited.ts', 30),
298+
uncoveredRecord('src/moved-pure.ts', 30),
299+
].join('\n'),
300+
),
301+
fileLines: () => null,
302+
});
303+
// 2 ordinary edits + 3 edited move lines; the 30-line pure move owes nothing.
304+
assert.equal(result.totalLines, 5);
305+
assert.deepEqual(
306+
result.offenders.map((f) => [f.path, f.uncoveredLines]),
307+
[
308+
['src/mod.ts', [2, 3]],
309+
['src/moved-edited.ts', [3, 7, 9]],
310+
],
311+
);
312+
});

scripts/coverage-changed/run.test.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -147,3 +147,14 @@ test('errors when the lcov report is missing rather than silently passing', () =
147147
assert.equal(code, 1);
148148
assert.match(out, /no lcov report/);
149149
});
150+
151+
test('a pure move owes nothing regardless of the host diff.renames setting', () => {
152+
git('config', 'diff.renames', 'false');
153+
git('mv', 'src/base.ts', 'src/moved.ts');
154+
git('commit', '-q', '-m', 'move');
155+
writeLcov('SF:src/moved.ts\nDA:1,0\nend_of_record\n');
156+
const { code, out } = capture(() => run(['--base', 'main'], repo));
157+
assert.equal(code, 0);
158+
assert.match(out, /Changed-line coverage gate: PASS/);
159+
assert.match(out, /0\/0 \(n\/a\)/);
160+
});

scripts/coverage-changed/run.ts

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,9 @@
33
//
44
// Reuses the lcov report that `pnpm test:coverage` already produced (never runs
55
// coverage a second time), joins it with `git diff --unified=0 <base>...HEAD`,
6-
// and fails when changed-line coverage is below the threshold. A PR carrying
6+
// and fails when changed-line coverage is below the threshold. Renames are
7+
// detected at RENAME_SIMILARITY regardless of the host's `diff.renames`, so a
8+
// moved file owes only the hunks that differ from its source. A PR carrying
79
// the `coverage-waiver` label skips the failure but still prints every number.
810
// The same markdown report goes to stdout and to the GitHub job summary; it
911
// includes changed-branch coverage and the count of changed executable lines
@@ -23,6 +25,7 @@ import {
2325

2426
const USAGE = 'Usage: pnpm check:coverage-changed [--base <ref>]\n';
2527
const LCOV_PATH = 'coverage/lcov.info';
28+
const RENAME_SIMILARITY = '90%';
2629
const GIT_DIFF_MAX_BUFFER_BYTES = 16 * 1024 * 1024;
2730

2831
function fmtPct(pct: number | null): string {
@@ -72,10 +75,11 @@ export function run(argv: readonly string[], cwd?: string): number {
7275
return 1;
7376
}
7477

75-
const diff = runCmdSync('git', ['diff', '--unified=0', '--no-color', `${base}...HEAD`], {
76-
cwd: root,
77-
maxBuffer: GIT_DIFF_MAX_BUFFER_BYTES,
78-
}).stdout;
78+
const diff = runCmdSync(
79+
'git',
80+
['diff', '--unified=0', '--no-color', `--find-renames=${RENAME_SIMILARITY}`, `${base}...HEAD`],
81+
{ cwd: root, maxBuffer: GIT_DIFF_MAX_BUFFER_BYTES },
82+
).stdout;
7983
const result = computeChangedCoverage({
8084
diffs: parseUnifiedDiff(diff),
8185
coverage: parseLcov(fs.readFileSync(lcovPath, 'utf8'), root),

0 commit comments

Comments
 (0)