Skip to content
Merged
Show file tree
Hide file tree
Changes from all 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
5 changes: 5 additions & 0 deletions .changeset/pack-test-behavior-notes.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"uptide": patch
---

`uptide pack test` scores each expected site at the severity of the rule or behavior note it belongs to, so a missed note that only needs a look, or a decision, no longer counts as a missed break. Behavior-note sites get their own precision and recall in the output and in `verification.json` (`behavior`, with the counts behind each rate), false negatives show their severity, and a rate with nothing to measure prints `n/a` instead of 100%, in `pack test` and in the README's packs table.
5 changes: 5 additions & 0 deletions .changeset/pack-truth-insertion-anchors.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"uptide": patch
---

Ground truth can name a site for an upgrade that only adds lines inside an existing call, such as an option added to a constructor: the site is the call's start line, so a pack finding there scores as a true positive instead of a false positive. `pnpm packs:truth` drafts these anchors from insert-only hunks for review, and docs/packs.md and CONTRIBUTING describe the rule.
33 changes: 31 additions & 2 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -132,14 +132,18 @@ open a **Pack request** issue, and say you are writing it.
Add `--also <package>@<version>` for each package the upgrade commit moved together with
it (its lockfile has the versions).
Read each site, drop what is not about the package, and give each its rule (`generic`
when no rule of the pack covers it). (15 minutes)
when no rule of the pack covers it). An option the upgrade only added inside an existing
call is drafted under `anchors`, at the call's start line; keep it like any other site.
(15 minutes)
6. **Score it.**
```sh
pnpm uptide pack test <package> # fetches the repositories once, scripts off
pnpm uptide pack test <package> --write # records the status in verification.json
```
Every false positive among breaking findings fails it. Fix the rule, not the ground truth.
A false negative is a site to cover or a known gap to say in the pull request.
A false negative is a site to cover or a known gap to say in the pull request. A site
expected under a behavior note scores at the note's `severity`, and behavior-note sites
get their own precision and recall: a missed "needs a look" is not a missed break.
7. **Smoke-test locally** without editing `verification.json`:
```sh
pnpm uptide check <package> --allow-candidate --cwd /path/to/local-fixture
Expand All @@ -154,6 +158,31 @@ open a **Pack request** issue, and say you are writing it.

`pnpm uptide` runs the CLI from source, so nothing needs building while you work.

## Run pack test locally

`pnpm uptide pack test` installs each ground-truth repository with the package manager its
lockfile names, scripts off. For a full run:

- **Corepack for Yarn only.** Repositories that pin Yarn Berry in `packageManager` (tldraw,
`yarn@4.7.0`) fail under Yarn 1. Run `corepack enable yarn`, not `corepack enable`: that
also replaces your global pnpm with a shim, and tests that run pnpm outside this checkout
then try to download a pnpm of corepack's choosing.
- **Where Yarn 4 comes from.** Corepack fetches Yarn 2 and later from repo.yarnpkg.com. Where
that host is blocked (a proxy, an allowlist), fetch it from npm instead:
`export COREPACK_NPM_REGISTRY=https://registry.npmjs.org COREPACK_ENABLE_DOWNLOAD_PROMPT=0`.
- **Disk.** Each repository stays cached with its dependencies installed, 2 to 8 GB per pack.
To score every pack on a bounded disk, give each pack its own caches and delete them after:
```sh
for p in ai zod; do # each pack you score
export UPTIDE_GROUND_TRUTH_CACHE=/tmp/uptide-truth/$p UPTIDE_CACHE_DIR=/tmp/uptide-cache/$p
pnpm uptide pack test $p
rm -rf "$UPTIDE_GROUND_TRUTH_CACHE" "$UPTIDE_CACHE_DIR"
done
```
Comparing two checkouts (a before and after table), score each pack in both before you
delete its caches. In-tree fixtures (`fixtures/repos/storefront`) are installed inside the
checkout itself, so run each checkout once without `--offline`.

## Rules that do not bend

These are what make Uptide safe to run on someone else's code. A pull request that breaks
Expand Down
42 changes: 32 additions & 10 deletions docs/packs.md
Original file line number Diff line number Diff line change
Expand Up @@ -198,8 +198,14 @@ Where the expected findings come from: two sources that are not Uptide.
the old side). One site per change: consecutive changed lines of the same change are one
site, at the first line of code (a line that is plainly another rule's starts a new
one); a run with a diagnostic inside it, or right after a diagnostic of the same rule, is
that diagnostic's site. Comment-only lines, pure insertions (no line at `<commit>`), and
changes for another package are not sites.
that diagnostic's site. Comment-only lines and changes for another package are not sites.
A pure insertion (no line at `<commit>`) is a site only when it adds to a call or object
the upgrade otherwise left unchanged, such as an option added to a constructor:
`new Redis(url, {` at line 138 that gains `protocol: 2` is a site at 138. The anchor is the
start line of the innermost call (or `new`) whose parentheses hold the inserted lines,
through any object and array literals, or of the object literal when no call takes it.
An insertion between statements or into a function body is not a site, nor is one into a
call the upgrade also changed: that call's changed lines are its site.

`pnpm packs:truth` drafts both for one repository:

Expand All @@ -209,8 +215,9 @@ pnpm packs:truth ai vercel/chatbot <commit> <upgrade-commit> --to 7.0.9 \
--also @ai-sdk/react@4.0.10 --also @ai-sdk/provider@4.0.1
```

It prints the compiler's new diagnostics with each line's text, and the changed and inserted
lines. You read each one, drop what is not about the package, and give every kept site its
It prints the compiler's new diagnostics with each line's text, the changed and inserted
lines, and `anchors`: each insertion into an unchanged call or object, at the line the rule
above anchors it, with the lines inserted there. You read each one, drop what is not about the package, and give every kept site its
rule, from its code and text. A site the pack has no rule for is expected under `generic`.
The expected findings never come from Uptide's output: a site `check` reports that neither
source has is a false positive, and it stays one.
Expand Down Expand Up @@ -255,10 +262,21 @@ findings:

- **per rule**: a site counts for a rule when file, line and rule all match;
- **overall**: at the line level, every severity;
- **behavior notes**: at the line level, the sites of the pack's behavior notes, whatever
their severity;
- **breaking**: at the line level, breaking findings only.

It prints precision and recall for each, and every false positive and false negative as
`repo file:line rule`, and sites found under another rule. It exits non-zero on any false
An expected site has the severity of the rule or note it is expected under: a rule's
`severity`; a note's `severity`, or breaking when the note reports findings without one (as
`check` reports them); `behavior` for a note `check` never reports as a finding (`decision`,
`test-follow-up`); breaking for `generic`. A predicted site has the severity `check` reported.
A missed site counts against breaking recall only when its severity is breaking: a missed
`deprecated` note ("needs a look") or a decision is not a missed break.

It prints precision and recall for each, `n/a` where there was nothing to measure (nothing
predicted, or nothing expected), every false positive as `repo file:line rule (severity)`,
every false negative the same way with the severity it was expected at, and sites found under
another rule. It exits non-zero on any false
positive among breaking findings, any fixture failure, or a `verification.json` that no
longer matches what it measured (`--write` records it).

Expand All @@ -277,7 +295,9 @@ breaking finding predicted. Anything else is a **candidate**:
cannot be published. Never edit `verification.json` just to smoke-test a pack.

The CLI reads the status from `verification.json`, which `pack test --write` writes and
which records the digest of the ground-truth file it was measured against. CI runs
which records the digest of the ground-truth file it was measured against, with precision,
recall and the counts behind them for breaking findings and for behavior-note sites
(`behavior`, absent from records written before it). The gate reads the breaking ones only. CI runs
`pack test` for every pack on every pull request that touches packs and fails when the
record no longer matches the evidence, so the label cannot outlive what supports it.

Expand Down Expand Up @@ -318,14 +338,16 @@ pnpm uptide pack test ai --write # record the measured status in verific
pnpm uptide pack test ai --fixtures-only --update-fixtures # write after.ts from the rules
```

`pack test` prints precision and recall per rule, overall and for breaking findings, and
every false positive and false negative as `repository file:line rule`. Exit codes: **0**
`pack test` prints precision and recall per rule, overall, for behavior-note sites and for
breaking findings, and every false positive and false negative as
`repository file:line rule (severity)`. Exit codes: **0**
every pack passed, **1** a false positive among breaking findings, a failing fixture, or a
`verification.json` the run does not support, **2** it could not run (not in a checkout, bad
arguments).

The "Verified packs" table in the README is generated from each pack's `verification.json`
and ground truth by `pnpm docs:packs`; CI fails when it is stale.
and ground truth by `pnpm docs:packs`; CI fails when it is stale. Its precision and recall are
the breaking ones, `n/a` when nothing breaking was predicted or expected.

### Config files and transactional edits

Expand Down
51 changes: 51 additions & 0 deletions packages/cli/src/pack.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
import { type PackTestReport, registeredPacks, unfinishedPack } from '@uptide/core';
import { expect, it } from 'vitest';
import { formatPackTest } from './pack.js';

it('prints behavior notes on their own row, n/a where nothing was predicted, and each miss with its severity', () => {
const entry = registeredPacks()[0];
if (!entry) throw new Error('no registered pack');
const tally = (tp: number, fp: number, fn: number) => ({
truePositives: tp,
falsePositives: fp,
falseNegatives: fn,
precision: tp + fp ? tp / (tp + fp) : 1,
recall: tp + fn ? tp / (tp + fn) : 1,
});
const site = (line: number, rule: string, severity: 'breaking' | 'deprecated') => ({
file: 'src/client.ts',
line,
rule,
severity,
});
const report: PackTestReport = {
...unfinishedPack(entry, ''),
problems: [],
repos: [
{
label: 'owner/name',
from: '5.0.0',
to: '6.0.0',
predicted: [site(13, 'client-defaults', 'deprecated')],
expected: [
site(9, 'client-defaults', 'deprecated'),
site(20, 'handshake-auth', 'breaking'),
],
falsePositives: [site(13, 'client-defaults', 'deprecated')],
falseNegatives: [
site(9, 'client-defaults', 'deprecated'),
site(20, 'handshake-auth', 'breaking'),
],
wrongRule: [],
},
],
overall: tally(0, 1, 2),
breaking: tally(0, 0, 1),
behavior: tally(0, 1, 2),
};
const out = formatPackTest(report);
expect(out).toMatch(/behavior notes \(sites\)\s+0%\s+0%\s+0\s+1\s+2/);
expect(out).toMatch(/breaking \(sites\)\s+n\/a\s+0%\s+0\s+0\s+1/);
expect(out).toContain('src/client.ts:9 client-defaults (deprecated)');
expect(out).toContain('src/client.ts:20 handshake-auth (breaking)');
});
15 changes: 10 additions & 5 deletions packages/cli/src/pack.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { existsSync, readFileSync } from 'node:fs';
import { dirname, join } from 'node:path';
import type { PackTestReport, RegisteredPack, Tally } from '@uptide/core';
import { type PackTestReport, type RegisteredPack, rateLabel, type Tally } from '@uptide/core';
import pc from 'picocolors';
import { CliError } from './errors.js';

Expand Down Expand Up @@ -48,10 +48,8 @@ export function packsToTest(
return [entry];
}

const pct = (n: number): string => `${Math.round(n * 100)}%`;

function row(name: string, t: Tally): string {
return ` ${name.padEnd(28)} ${pct(t.precision).padStart(9)} ${pct(t.recall).padStart(7)} ${String(t.truePositives).padStart(5)} ${String(t.falsePositives).padStart(4)} ${String(t.falseNegatives).padStart(4)}`;
return ` ${name.padEnd(28)} ${rateLabel(t, 'precision').padStart(9)} ${rateLabel(t, 'recall').padStart(7)} ${String(t.truePositives).padStart(5)} ${String(t.falsePositives).padStart(4)} ${String(t.falseNegatives).padStart(4)}`;
}

/** What `uptide pack test` prints for one pack. */
Expand Down Expand Up @@ -109,6 +107,8 @@ export function formatPackTest(report: PackTestReport, color = false): string {
for (const [rule, t] of Object.entries(report.rules).sort(([a], [b]) => a.localeCompare(b)))
out.push(row(rule, t));
out.push(row('overall (sites)', report.overall));
// Absent from a report written before behavior notes were scored on their own.
if (report.behavior) out.push(row('behavior notes (sites)', report.behavior));
out.push(c.bold(row('breaking (sites)', report.breaking)));
const listed = (
title: string,
Expand All @@ -133,7 +133,12 @@ export function formatPackTest(report: PackTestReport, color = false): string {
listed(
'false negatives',
report.repos.flatMap((r) =>
r.falseNegatives.map((s) => ({ label: r.label, file: s.file, line: s.line, text: s.rule })),
r.falseNegatives.map((s) => ({
label: r.label,
file: s.file,
line: s.line,
text: s.severity ? `${s.rule} (${s.severity})` : s.rule,
})),
),
);
listed(
Expand Down
10 changes: 10 additions & 0 deletions packages/core/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -186,8 +186,10 @@ export {
type PackStatus,
type PackVerification,
type RegisteredPack,
rateLabel,
replaceAtSite,
VERIFIED_MIN_REPOS,
type VerificationTally,
} from './packs/contract.js';
export { activePacks, packStatus, registeredPacks } from './packs/index.js';
export { leadName, leadNames, packLeading } from './packs/leads.js';
Expand All @@ -213,6 +215,14 @@ export {
type PinnedRepo,
truthCacheDir,
} from './packs/tooling/truth.js';
export {
type ChangedLine,
diffLines,
enclosingLiteral,
type InsertedLine,
type InsertionAnchor,
insertionAnchors,
} from './packs/tooling/truth-diff.js';
export type { MigrationPack, MigrationRule } from './packs/types.js';
export { zodPack } from './packs/zod/index.js';
export { type PlanOptions, type PlanServices, upgradePlan } from './plan/gather.js';
Expand Down
11 changes: 10 additions & 1 deletion packages/core/src/packs/ai/verification.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,15 @@
"breaking": {
"precision": 1,
"recall": 0.8182,
"falsePositives": 0
"falsePositives": 0,
"truePositives": 9,
"falseNegatives": 2
},
"behavior": {
"precision": 1,
"recall": 1,
"falsePositives": 0,
"truePositives": 0,
"falseNegatives": 0
}
}
34 changes: 33 additions & 1 deletion packages/core/src/packs/contract.ts
Original file line number Diff line number Diff line change
Expand Up @@ -157,7 +157,39 @@ export interface PackVerification {
truth: string;
/** Public repositories in the ground truth (fixtures excluded). */
repos: number;
breaking: { precision: number; recall: number; falsePositives: number };
breaking: VerificationTally;
/** Sites of behavior notes, whatever their severity. Absent from records written before it. */
behavior?: VerificationTally;
}

/**
* Precision and recall as recorded. A ratio with nothing to measure reads 1; the counts, absent
* from records written before they were kept, tell that apart from a perfect score.
*/
export interface VerificationTally {
precision: number;
recall: number;
falsePositives: number;
truePositives?: number;
falseNegatives?: number;
}

/** A run's tally as `verification.json` records it. */
export function verificationTally(t: Required<VerificationTally>): Required<VerificationTally> {
return {
precision: Number(t.precision.toFixed(4)),
recall: Number(t.recall.toFixed(4)),
falsePositives: t.falsePositives,
truePositives: t.truePositives,
falseNegatives: t.falseNegatives,
};
}

/** A precision or recall for people: `n/a` when nothing was predicted, or nothing expected. */
export function rateLabel(tally: VerificationTally, rate: 'precision' | 'recall'): string {
const missed = rate === 'precision' ? tally.falsePositives : tally.falseNegatives;
if (tally.truePositives === 0 && missed === 0) return 'n/a';
return `${Math.round(tally[rate] * 100)}%`;
}

/**
Expand Down
11 changes: 10 additions & 1 deletion packages/core/src/packs/lucide-react/verification.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,15 @@
"breaking": {
"precision": 1,
"recall": 1,
"falsePositives": 0
"falsePositives": 0,
"truePositives": 11,
"falseNegatives": 0
},
"behavior": {
"precision": 1,
"recall": 1,
"falsePositives": 0,
"truePositives": 0,
"falseNegatives": 0
}
}
11 changes: 10 additions & 1 deletion packages/core/src/packs/next/verification.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,15 @@
"breaking": {
"precision": 1,
"recall": 0.8462,
"falsePositives": 0
"falsePositives": 0,
"truePositives": 11,
"falseNegatives": 2
},
"behavior": {
"precision": 1,
"recall": 1,
"falsePositives": 0,
"truePositives": 0,
"falseNegatives": 0
}
}
11 changes: 10 additions & 1 deletion packages/core/src/packs/react/verification.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,15 @@
"breaking": {
"precision": 1,
"recall": 0.6899,
"falsePositives": 0
"falsePositives": 0,
"truePositives": 89,
"falseNegatives": 40
},
"behavior": {
"precision": 1,
"recall": 1,
"falsePositives": 0,
"truePositives": 0,
"falseNegatives": 0
}
}
11 changes: 10 additions & 1 deletion packages/core/src/packs/sentry-browser/verification.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,15 @@
"breaking": {
"precision": 1,
"recall": 1,
"falsePositives": 0
"falsePositives": 0,
"truePositives": 3,
"falseNegatives": 0
},
"behavior": {
"precision": 1,
"recall": 1,
"falsePositives": 0,
"truePositives": 0,
"falseNegatives": 0
}
}
Loading
Loading