From 23fd1a76422cb0201f7d153b79c5f6ff8efff4ef Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 9 Oct 2026 07:33:01 +0000 Subject: [PATCH] feat(check): one finding at a repository parameter that call sites in several workspaces trip A parameter cause clustered per workspace compile only, so useTransform(ref) in tldraw, rejected once in apps/examples and once in packages/tldraw by packages/editor's useTransform.ts:6, stayed two findings at the call sites. A lone site now carries the parameter as sharedCause; merging workspaces joins lone sites to the anchor reported for the same declaration, or makes one from two or more of them. The anchor counts its call sites (callSites, additive in the JSON report); check --details and the HTML report print N call sites under it. pack test scores the anchor at the declaration and treats the call sites under it as evidence, never as false positives. Co-Authored-By: Claude Opus 5.5 Claude-Session: https://claude.ai/code/session_01AKRx4kuV6oi25MP5WguxW9 Signed-off-by: Claude --- .changeset/check-shared-root-findings.md | 13 ++ docs/architecture.md | 10 +- packages/cli/src/format-check-details.test.ts | 42 ++++++ packages/cli/src/format-check-details.ts | 4 +- packages/cli/src/html/render.test.ts | 48 +++++++ packages/cli/src/html/render.ts | 27 +++- .../src/adapters/typescript/cause.test.ts | 11 ++ .../core/src/adapters/typescript/compile.ts | 2 + packages/core/src/check/check.test.ts | 70 ++++++++++ packages/core/src/check/check.ts | 6 + packages/core/src/check/shared-root.test.ts | 124 ++++++++++++++++++ packages/core/src/check/shared-root.ts | 103 +++++++++++++++ packages/core/src/check/unattributed.ts | 101 +++++++------- packages/core/src/domain/report.ts | 20 ++- packages/core/src/domain/usage.ts | 6 + .../core/src/packs/tooling/pack-test.test.ts | 50 ++++++- packages/core/src/packs/tooling/pack-test.ts | 25 ++++ 17 files changed, 612 insertions(+), 50 deletions(-) create mode 100644 .changeset/check-shared-root-findings.md create mode 100644 packages/core/src/check/shared-root.test.ts create mode 100644 packages/core/src/check/shared-root.ts diff --git a/.changeset/check-shared-root-findings.md b/.changeset/check-shared-root-findings.md new file mode 100644 index 00000000..74bb8602 --- /dev/null +++ b/.changeset/check-shared-root-findings.md @@ -0,0 +1,13 @@ +--- +"uptide": minor +--- + +**One finding for one edit, across workspaces.** When call sites in different workspaces of a +repository fail because of the same local declaration (a hook's parameter typed +`RefObject` that `useRef` no longer satisfies), `check` now reports one breaking +finding at that declaration and lists the call sites under it as evidence, as it already did +when two sites in one workspace tripped it. The JSON report adds `callSites` to such an anchor +(how many call sites trip it, every workspace counted; `downstream` lists them) and `sharedCause` +to a lone site's finding before it is joined; nothing is renamed or removed. `check --details` +and the HTML report print `N call sites` under the anchor. `uptide pack test` scores the anchor +at the declaration; the call sites under it are neither true nor false positives. diff --git a/docs/architecture.md b/docs/architecture.md index c7b8689d..e9af8bfb 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -589,8 +589,14 @@ the target's `useRef` returns `RefObject`) is anchored at th `anchorOnly`, when at least two sites trip it: the one edit is the parameter's type, as the migration guides say, and the call sites are evidence. The parameter may sit in another workspace whose source the program maps (`packages/editor` for a call in `packages/tldraw`); -its path is the repository's. A parameter one site trips is left to -that site, which may as well be the argument's. +its path is the repository's. A parameter one site trips in a workspace is left to that site +there, carrying the parameter as a candidate (`sharedCause`). When the workspaces' reports are +merged (`mergeAcrossWorkspaces`, `check/shared-root.ts`), a lone site joins the anchor another +workspace already reports for the same declaration, and lone sites of two or more workspaces +make one anchor: one breaking finding at the declaration, `callSites` counting every workspace's +call sites, `downstream` listing them. A parameter only one site in the whole repository trips +stays that site's own finding. `pack test` scores the anchor at the declaration; the call sites +under it are evidence, neither true nor false positives. A diagnostic confirms exactly one usage: the innermost whose span contains the diagnostic's start. `z.string().trim().url()` is three usages on one line, and an error diff --git a/packages/cli/src/format-check-details.test.ts b/packages/cli/src/format-check-details.test.ts index 823b3079..70199270 100644 --- a/packages/cli/src/format-check-details.test.ts +++ b/packages/cli/src/format-check-details.test.ts @@ -131,6 +131,48 @@ const report: CheckReport = { }; describe('formatCheckDetails', () => { + it('says how many call sites trip a parameter anchor, and shows them as evidence', () => { + const anchored = finding({ + fixability: 'assisted', + anchorOnly: true, + callSites: 2, + reason: 'whose parameter `ref` no longer accepts what the target gives it', + change: { package: 'react', path: 'cause:ref', kind: 'cause' }, + usage: { file: 'packages/editor/src/lib/hooks/useTransform.ts', line: 6 }, + downstream: [ + { + file: 'apps/examples/src/x.tsx', + line: 11, + code: 2345, + message: 'Argument not assignable.', + }, + { + file: 'packages/tldraw/src/y.tsx', + line: 59, + code: 2345, + message: 'Argument not assignable.', + }, + ], + }); + const out = formatCheck( + { + ...report, + packages: [ + { + ...(report.packages[0] as PackageReport), + name: 'react', + findings: [anchored], + }, + ], + }, + { color: false }, + ); + expect(out).toContain('packages/editor/src/lib/hooks/useTransform.ts:6 `ref`'); + expect(out).toContain('2 call sites trip it at the target, for example'); + expect(out).toContain('apps/examples/src/x.tsx:11 TS2345: Argument not assignable.'); + expect(out).toContain('packages/tldraw/src/y.tsx:59 TS2345: Argument not assignable.'); + }); + it('renders every site with its snippet and reason', () => { const out = formatCheck(report, { color: false }); expect(out).toBe( diff --git a/packages/cli/src/format-check-details.ts b/packages/cli/src/format-check-details.ts index e90e2d91..d1780fa0 100644 --- a/packages/cli/src/format-check-details.ts +++ b/packages/cli/src/format-check-details.ts @@ -99,7 +99,9 @@ function siteLines( ` ${tint(mark)} ${f.usage.file}:${f.usage.line} \`${f.change.path.replace(/^cause:/, '')}\``, ` ${f.reason}`, colors.dim( - ` evidence: ${plural(sites.length, 'error')} at the target${shown.length ? ', for example' : ''}`, + f.callSites !== undefined + ? ` ${plural(f.callSites, 'call site')} trip it at the target${shown.length ? ', for example' : ''}` + : ` evidence: ${plural(sites.length, 'error')} at the target${shown.length ? ', for example' : ''}`, ), ...shown.map((site) => colors.dim(` ${site.file}:${site.line} TS${site.code}: ${site.message}`), diff --git a/packages/cli/src/html/render.test.ts b/packages/cli/src/html/render.test.ts index 6fb4c5f0..1fae708d 100644 --- a/packages/cli/src/html/render.test.ts +++ b/packages/cli/src/html/render.test.ts @@ -115,6 +115,54 @@ describe('HTML report', () => { expect(html.match(/<\/script>/g)).toHaveLength(1); expect(html).not.toContain(' { + const report = fixture('zod'); + const p = report.packages[0]; + assert(p); + const decl = { file: 'packages/editor/src/lib/hooks/useTransform.ts', line: 6 }; + p.findings.push({ + change: { + package: 'zod', + from: p.installed, + to: p.target, + path: 'cause:ref', + kind: 'cause', + severity: 'breaking', + source: 'types', + confidence: 1, + }, + usage: { + ...decl, + column: 1, + endLine: 6, + endColumn: 1, + symbolPath: 'cause:ref', + access: 'read', + snippet: '', + via: 'inferred', + }, + anchorOnly: true, + callSites: 2, + downstream: [ + { file: 'apps/examples/src/x.tsx', line: 11, code: 2345, message: 'x' }, + { file: 'packages/tldraw/src/y.tsx', line: 59, code: 2345, message: 'x' }, + ], + severity: 'breaking', + confidence: 1, + fixability: 'assisted', + reason: 'whose parameter `ref` no longer accepts what the target gives it', + }); + const g = p.plan?.[0]; + assert(g); + g.locations = [decl]; + const html = renderHtml(report, opts); + expect(html).toContain('

2 call sites

'); + expect(html).toContain('
  • apps/examples/src/x.tsx:11 TS2345
    • '); + expect(html).toContain( + '
  • packages/tldraw/src/y.tsx:59 TS2345
    • ', + ); + }); + it('caps excerpts at 50 per group but retains additional sites within the size budget', () => { const report = fixture('zod'); const g = report.packages[0]?.plan?.[0]; diff --git a/packages/cli/src/html/render.ts b/packages/cli/src/html/render.ts index ef59dc77..2cd6951c 100644 --- a/packages/cli/src/html/render.ts +++ b/packages/cli/src/html/render.ts @@ -47,6 +47,30 @@ function compilerAt(p: PackageReport, group: PlanGroup, file: string, line: numb ].join(' ยท ') || (group.severity !== 'deprecated' ? group.detail : '') ); } +/** + * Under a parameter anchor: `N call sites`, then where (the first few). The anchor is the one + * edit; the call sites, in whatever workspace, are its evidence. + */ +function callSitesAt( + p: PackageReport, + file: string, + line: number, +): ((e: (s: unknown) => string) => string) | undefined { + const anchor = p.findings.find( + (f) => + f.change.kind === 'cause' && + f.callSites !== undefined && + f.usage.file === file && + f.usage.line === line, + ); + if (!anchor?.callSites) return undefined; + const sites = anchor.downstream ?? []; + const shown = sites.slice(0, CALL_SITES_SHOWN); + return (e) => + `

    ${e(anchor.callSites)} ${anchor.callSites === 1 ? 'call site' : 'call sites'}

      ${shown.map((s) => `
    • ${e(`${s.file}:${s.line}`)} TS${e(s.code)}
      • `).join('')}${sites.length > shown.length ? `
    • and ${e(sites.length - shown.length)} more
      • ` : ''}
    `; +} +const CALL_SITES_SHOWN = 10; + /** No repo data is interpolated into executable JS, CSS, attribute names or element names. */ export function renderHtml(report: CheckReport, opts: HtmlOptions): string { function render( @@ -108,7 +132,8 @@ export function renderHtml(report: CheckReport, opts: HtmlOptions): string { ? `
    compiler messageSame message, shown above
    ` : `
    compiler message
    ${escapeHtml(message)}
    `; } - const article = `
    ${link}
    ${excerpt}${compiler}
    `; + const callers = callSitesAt(p, site.file, site.line); + const article = `
    ${link}
    ${excerpt}${compiler}${callers ? callers(e) : ''}
    `; if (index < siteLimit) body += article; else overflow += article; } diff --git a/packages/core/src/adapters/typescript/cause.test.ts b/packages/core/src/adapters/typescript/cause.test.ts index 84e74e86..30924120 100644 --- a/packages/core/src/adapters/typescript/cause.test.ts +++ b/packages/core/src/adapters/typescript/cause.test.ts @@ -99,6 +99,7 @@ describe('root cause guard', () => { (d) => d.file === 'src/param.ts' && d.code === 2345, ); expect(mismatches.map((d) => d.line)).toEqual([5, 6]); + for (const d of mismatches) expect(d.sharedCause).toBeUndefined(); for (const d of mismatches) expect(d.cause).toEqual({ name: 'b', @@ -126,5 +127,15 @@ describe('root cause guard', () => { const at5 = signal.diagnostics.find((d) => d.file === 'src/single.ts' && d.line === 5); expect(at5?.code).toBe(2345); expect(at5?.cause?.anchorOnly).toBeUndefined(); + // Not a cause in this workspace, but offered to the others: another workspace's lone site + // that trips the same parameter makes one finding at it (check/shared-root.ts). + expect(at5?.sharedCause).toEqual({ + name: 'b', + file: 'src/single.ts', + line: 2, + reason: + "whose parameter `b: Box` no longer accepts what the target gives it; widen the parameter's type there", + anchorOnly: true, + }); }); }); diff --git a/packages/core/src/adapters/typescript/compile.ts b/packages/core/src/adapters/typescript/compile.ts index 0680b3e2..c272763f 100644 --- a/packages/core/src/adapters/typescript/compile.ts +++ b/packages/core/src/adapters/typescript/compile.ts @@ -924,6 +924,8 @@ export async function compileAgainstTargets( : undefined) ?? findCause(programs, d, overlaid, repo.dir, erroredLines); if (cause) diagnostic.cause = cause; + // A parameter only this site trips: left to the site here, offered to the other workspaces. + else if (parameter !== undefined) diagnostic.sharedCause = byParameter.get(parameter); return diagnostic; }); return { diff --git a/packages/core/src/check/check.test.ts b/packages/core/src/check/check.test.ts index 67b8dc4a..3929d88e 100644 --- a/packages/core/src/check/check.test.ts +++ b/packages/core/src/check/check.test.ts @@ -414,6 +414,76 @@ describe('mergeAcrossWorkspaces', () => { ...over, }); + it('makes one finding at a repository parameter that one call site trips in each of two workspaces', () => { + // tldraw: useTransform(rSvg) in apps/examples and in packages/tldraw, both rejected by the + // hook's parameter in packages/editor. Each workspace has one site; together they are one edit. + const lone = (file: string, line: number): Finding => ({ + change: { + package: 'react', + from: '18.3.1', + to: '19.2.1', + path: 'TS2345', + kind: 'type', + severity: 'breaking', + source: 'types', + confidence: 1, + evidence: 'checker', + }, + usage: { + file, + line, + column: 15, + endLine: line, + endColumn: 19, + symbolPath: 'TS2345', + access: 'read', + snippet: 'useTransform(rSvg, x, y)', + via: 'inferred', + compileError: "Argument of type 'RefObject' is not assignable.", + }, + sharedCause: { + name: 'ref', + file: '../../packages/editor/src/lib/hooks/useTransform.ts', + line: 6, + reason: + 'whose parameter `ref: React.RefObject` no longer accepts what the target gives it', + anchorOnly: true, + }, + severity: 'breaking', + confidence: 1, + fixability: 'unknown', + reason: 'compile error not attributed to a known API change', + }); + const [merged] = mergeAcrossWorkspaces( + [ + base('apps/examples', 'react', { + status: 'breaking', + findings: [lone('src/examples/CustomComponentsExample.tsx', 11)], + }), + base('packages/tldraw', 'react', { + status: 'breaking', + findings: [lone('src/lib/canvas/TldrawSelectionForeground.tsx', 59)], + }), + ], + {}, + ); + expect(merged?.findings).toHaveLength(1); + const anchor = merged?.findings[0] as Finding; + expect(anchor).toMatchObject({ + change: { kind: 'cause', path: 'cause:ref' }, + usage: { file: 'packages/editor/src/lib/hooks/useTransform.ts', line: 6 }, + anchorOnly: true, + callSites: 2, + severity: 'breaking', + }); + expect(anchor.downstream?.map((d) => `${d.file}:${d.line}`)).toEqual([ + 'apps/examples/src/examples/CustomComponentsExample.tsx:11', + 'packages/tldraw/src/lib/canvas/TldrawSelectionForeground.tsx:59', + ]); + expect(anchor.sharedCause).toBeUndefined(); + expect(sitesOf(anchor)).toBe(1); + }); + it('merges the same dependency at the same versions across workspaces, and marks catalog entries', () => { const out = mergeAcrossWorkspaces( [ diff --git a/packages/core/src/check/check.ts b/packages/core/src/check/check.ts index 083f627f..afd1709d 100644 --- a/packages/core/src/check/check.ts +++ b/packages/core/src/check/check.ts @@ -54,6 +54,7 @@ import { mapWithLimit, mapWithSerialRetry } from './pool.js'; import { isBehind, rankCandidates } from './rank.js'; import { groupRootCauses } from './root-cause.js'; import { runtimeChangeFindings } from './runtime-changes.js'; +import { joinSharedRoots } from './shared-root.js'; import { confirmBreaking, evidenceOf, tierOf } from './tier.js'; import { typesPackageOf, typesReleaseFor } from './types-release.js'; import { unattributedFindings } from './unattributed.js'; @@ -572,6 +573,9 @@ export function mergeAcrossWorkspaces( p.findings.map((f) => ({ ...f, usage: { ...f.usage, file: prefixed(p, f.usage.file) }, + ...(f.sharedCause + ? { sharedCause: { ...f.sharedCause, file: prefixed(p, f.sharedCause.file) } } + : {}), // A cluster's description names its cause by file: the same prefix applies there. change: f.change.path.startsWith('cause:') ? { @@ -605,6 +609,8 @@ export function mergeAcrossWorkspaces( runtimeMs: sum((p) => p.timing.runtimeMs ?? 0), }, }; + // Sites that trip one repository declaration from different workspaces are one edit there. + combined.findings = joinSharedRoots(combined.findings); if (first.runtime) combined.runtime = first.runtime; // What moves with the package is one plan for the repository, carried by whichever // workspace reports it; the merged entry keeps every companion and conflict named. diff --git a/packages/core/src/check/shared-root.test.ts b/packages/core/src/check/shared-root.test.ts new file mode 100644 index 00000000..d7ff9280 --- /dev/null +++ b/packages/core/src/check/shared-root.test.ts @@ -0,0 +1,124 @@ +import { describe, expect, it } from 'vitest'; +import type { Finding } from '../domain/report.js'; +import type { DiagnosticCause } from '../domain/usage.js'; +import { joinSharedRoots } from './shared-root.js'; + +const cause: DiagnosticCause = { + name: 'ref', + file: 'packages/editor/src/lib/hooks/useTransform.ts', + line: 6, + reason: + 'whose parameter `ref: RefObject` no longer accepts what the target gives it', + anchorOnly: true, +}; +const meta = { package: 'react', from: '18.3.1', to: '19.2.1' }; + +const lone = (file: string, line: number, over: Partial = {}): Finding => ({ + change: { + ...meta, + path: 'TS2345', + kind: 'type', + severity: 'breaking', + source: 'types', + confidence: 1, + }, + usage: { + file, + line, + column: 1, + endLine: line, + endColumn: 1, + symbolPath: 'TS2345', + access: 'read', + snippet: '', + via: 'inferred', + compileError: 'Argument of type X is not assignable to parameter of type Y.', + }, + sharedCause: cause, + severity: 'breaking', + confidence: 1, + fixability: 'unknown', + reason: 'compile error not attributed to a known API change', + ...over, +}); + +/** The anchor one workspace already reports (two of its own sites trip the parameter). */ +const anchor: Finding = { + change: { + ...meta, + path: 'cause:ref', + kind: 'cause', + severity: 'breaking', + source: 'types', + confidence: 1, + }, + usage: { + file: cause.file, + line: cause.line, + column: 1, + endLine: cause.line, + endColumn: 1, + symbolPath: 'cause:ref', + access: 'read', + snippet: '', + via: 'inferred', + }, + downstream: [ + { file: 'packages/editor/src/lib/a.tsx', line: 3, code: 2345, message: 'x' }, + { file: 'packages/editor/src/lib/b.tsx', line: 4, code: 2345, message: 'x' }, + ], + anchorOnly: true, + callSites: 2, + severity: 'breaking', + confidence: 1, + fixability: 'assisted', + reason: cause.reason, +}; + +describe('joinSharedRoots', () => { + it('adds a lone site of another workspace to the anchor already reported for the declaration', () => { + const out = joinSharedRoots([anchor, lone('apps/examples/src/x.tsx', 11)]); + expect(out).toHaveLength(1); + expect(out[0]?.callSites).toBe(3); + expect(out[0]?.downstream?.map((d) => `${d.file}:${d.line}`)).toEqual([ + 'packages/editor/src/lib/a.tsx:3', + 'packages/editor/src/lib/b.tsx:4', + 'apps/examples/src/x.tsx:11', + ]); + expect(out[0]?.change.notes).toContain('3 errors caused by `ref`'); + }); + + it('makes one anchor of two lone sites in different workspaces, under the rule that claimed them', () => { + const out = joinSharedRoots([ + lone('apps/examples/src/x.tsx', 11, { rule: 'ref-object-nullable' }), + lone('packages/tldraw/src/y.tsx', 59, { rule: 'ref-object-nullable' }), + ]); + expect(out).toHaveLength(1); + expect(out[0]).toMatchObject({ + usage: { file: cause.file, line: 6 }, + anchorOnly: true, + callSites: 2, + rule: 'ref-object-nullable', + }); + }); + + it('leaves a site no other shares as its own finding, without the candidate', () => { + const other = lone('apps/examples/src/x.tsx', 11); + const out = joinSharedRoots([other]); + expect(out).toHaveLength(1); + expect(out[0]?.usage.file).toBe('apps/examples/src/x.tsx'); + expect(out[0]?.sharedCause).toBeUndefined(); + }); + + it('keeps sites of different declarations apart', () => { + const elsewhere = { ...cause, file: 'packages/editor/src/lib/hooks/other.ts' }; + const out = joinSharedRoots([ + lone('apps/examples/src/x.tsx', 11), + lone('packages/tldraw/src/y.tsx', 59, { sharedCause: elsewhere }), + ]); + expect(out.map((f) => `${f.usage.file}:${f.usage.line}`)).toEqual([ + 'apps/examples/src/x.tsx:11', + 'packages/tldraw/src/y.tsx:59', + ]); + }); +}); diff --git a/packages/core/src/check/shared-root.ts b/packages/core/src/check/shared-root.ts new file mode 100644 index 00000000..ed657cf9 --- /dev/null +++ b/packages/core/src/check/shared-root.ts @@ -0,0 +1,103 @@ +import type { Finding } from '../domain/report.js'; +import type { CompileDiagnostic, DiagnosticCause } from '../domain/usage.js'; +import { clusterFinding, describeCluster } from './unattributed.js'; + +const keyOf = (c: Pick): string => + `${c.file}:${c.line}:${c.name}`; + +/** The cause an anchor finding stands for, read back from the finding. */ +function anchorKey(f: Finding): string | undefined { + if (f.change.kind !== 'cause' || !f.anchorOnly || !f.change.path.startsWith('cause:')) + return undefined; + return keyOf({ file: f.usage.file, line: f.usage.line, name: f.change.path.slice(6) }); +} + +/** A lone site as the compile error it was, repository-relative. */ +function asDiagnostic(f: Finding, cause: DiagnosticCause): CompileDiagnostic { + return { + file: f.usage.file, + line: f.usage.line, + column: f.usage.column, + endLine: f.usage.endLine, + endColumn: f.usage.endColumn, + code: Number(/^TS(\d+)$/.exec(f.change.path)?.[1] ?? 0), + message: f.usage.compileError ?? '', + snippet: f.usage.snippet, + cause, + }; +} + +/** + * Several workspaces, one edit. A repository parameter that one call site trips in each of + * several workspaces (`useTransform(ref)` in `apps/examples` and in `packages/tldraw`, both + * rejected by `packages/editor/src/lib/hooks/useTransform.ts:6`) is one breaking finding at the + * declaration, with the call sites under it as evidence, as one workspace with two such sites + * already is. A lone site joins the anchor another workspace already reports for the same + * declaration; two lone sites in different workspaces make one. A site no other shares stays + * its own finding. Files are repository-relative (`mergeAcrossWorkspaces` prefixes them first). + */ +export function joinSharedRoots(findings: Finding[]): Finding[] { + const candidates = new Map(); + for (const f of findings) { + if (!f.sharedCause) continue; + const key = keyOf(f.sharedCause); + candidates.set(key, [...(candidates.get(key) ?? []), f]); + } + const joined = new Set(); + const replaced = new Map(); + const created = new Map(); + for (const [key, lone] of candidates) { + const anchor = findings.find((f) => anchorKey(f) === key); + if (anchor) { + const cause = lone[0]?.sharedCause as DiagnosticCause; + const downstream = [ + ...(anchor.downstream ?? []), + ...lone.map((f) => { + const d = asDiagnostic(f, cause); + return { + file: d.file, + line: d.line, + code: d.code, + message: d.message.split('\n')[0] ?? '', + }; + }), + ]; + replaced.set(anchor, { + ...(replaced.get(anchor) ?? anchor), + downstream, + callSites: downstream.length, + change: { + ...anchor.change, + notes: describeCluster(downstream.length, { + ...cause, + file: anchor.usage.file, + line: anchor.usage.line, + }), + }, + }); + for (const f of lone) joined.add(f); + continue; + } + if (lone.length < 2) continue; + const first = lone[0] as Finding; + const cause = first.sharedCause as DiagnosticCause; + const meta = { package: first.change.package, from: first.change.from, to: first.change.to }; + const group = lone.map((f) => asDiagnostic(f, cause)); + const cluster = clusterFinding(group, meta); + // The rule that claimed the lone sites claims the anchor too. + created.set(first, first.rule ? { ...cluster, rule: first.rule } : cluster); + for (const f of lone) joined.add(f); + } + const out: Finding[] = []; + for (const f of findings) { + const made = created.get(f); + if (made) out.push(made); + if (joined.has(f)) continue; + const kept = replaced.get(f) ?? f; + if (kept.sharedCause) { + const { sharedCause: _unshared, ...rest } = kept; + out.push(rest); + } else out.push(kept); + } + return out; +} diff --git a/packages/core/src/check/unattributed.ts b/packages/core/src/check/unattributed.ts index 3b660bcb..c1d8ff5d 100644 --- a/packages/core/src/check/unattributed.ts +++ b/packages/core/src/check/unattributed.ts @@ -55,53 +55,65 @@ export function unattributedFindings( list.push(d); clusters.set(key, list); } - const clustered: Finding[] = [...clusters.values()].map((group) => { - const cause = (group[0] as CompileDiagnostic).cause as DiagnosticCause; - const usage: Usage = { - file: cause.file, - line: cause.line, - column: 1, - endLine: cause.line, - endColumn: 1, - symbolPath: `cause:${cause.name}`, - access: 'read', - snippet: '', - via: 'inferred', - }; - const change: Change = { - ...meta, - path: `cause:${cause.name}`, - kind: 'cause', - severity: 'breaking', - source: 'types', - confidence: 1, - evidence: 'checker', - notes: describeCluster(group.length, { - ...cause, - file: posix.normalize(`${filePrefix}${cause.file}`), - }), - }; - return { - change, - usage, - downstream: group.map((d) => ({ - file: posix.normalize(`${filePrefix}${d.file}`), - line: d.line, - code: d.code, - message: d.message.split('\n')[0] ?? d.message, - })), - // A cause that is itself the one edit (a compiler option, a parameter's type) is the - // site; the errors under it are evidence. - ...(cause.anchorOnly || cause.config ? { anchorOnly: true as const } : {}), - severity: 'breaking', - confidence: 1, - fixability: cause.anchorOnly || cause.config ? 'assisted' : 'unknown', - reason: cause.anchorOnly || cause.config ? cause.reason : UNATTRIBUTED_REASON, - }; - }); + const clustered: Finding[] = [...clusters.values()].map((group) => + clusterFinding(group, meta, filePrefix), + ); return [...clustered, ...plainFindings(single, meta, surfaceB, unresolvedFiles)]; } +/** One finding at a traced cause, with the errors it explains under it. */ +export function clusterFinding( + group: CompileDiagnostic[], + meta: { package: string; from: string; to: string }, + /** Prefix that makes a workspace-relative file repository-relative (`packages/api/`). */ + filePrefix = '', +): Finding { + const cause = (group[0] as CompileDiagnostic).cause as DiagnosticCause; + const usage: Usage = { + file: cause.file, + line: cause.line, + column: 1, + endLine: cause.line, + endColumn: 1, + symbolPath: `cause:${cause.name}`, + access: 'read', + snippet: '', + via: 'inferred', + }; + const change: Change = { + ...meta, + path: `cause:${cause.name}`, + kind: 'cause', + severity: 'breaking', + source: 'types', + confidence: 1, + evidence: 'checker', + notes: describeCluster(group.length, { + ...cause, + file: posix.normalize(`${filePrefix}${cause.file}`), + }), + }; + return { + change, + usage, + downstream: group.map((d) => ({ + file: posix.normalize(`${filePrefix}${d.file}`), + line: d.line, + code: d.code, + message: d.message.split('\n')[0] ?? d.message, + })), + // A cause that is itself the one edit (a compiler option, a parameter's type) is the + // site; the errors under it are evidence. + ...(cause.anchorOnly || cause.config ? { anchorOnly: true as const } : {}), + // A parameter's errors are its call sites: the report says how many. + ...(cause.anchorOnly && !cause.config ? { callSites: group.length } : {}), + severity: 'breaking', + confidence: 1, + fixability: cause.anchorOnly || cause.config ? 'assisted' : 'unknown', + reason: cause.anchorOnly || cause.config ? cause.reason : UNATTRIBUTED_REASON, + }; +} + function plainFindings( diagnostics: CompileDiagnostic[], meta: { package: string; from: string; to: string }, @@ -142,6 +154,7 @@ function plainFindings( return { change, usage, + ...(d.sharedCause ? { sharedCause: d.sharedCause } : {}), severity: inconclusive ? 'unverified' : 'breaking', confidence: 1, fixability: 'unknown', diff --git a/packages/core/src/domain/report.ts b/packages/core/src/domain/report.ts index 23de32f5..1e131dcc 100644 --- a/packages/core/src/domain/report.ts +++ b/packages/core/src/domain/report.ts @@ -1,7 +1,13 @@ import type { ErrorCode } from '../errors.js'; import type { Change, Severity } from './change.js'; import type { RuntimeChange, RuntimeLoad } from './runtime.js'; -import type { CompileCoverage, CompileDiagnostic, Unanalyzed, Usage } from './usage.js'; +import type { + CompileCoverage, + CompileDiagnostic, + DiagnosticCause, + Unanalyzed, + Usage, +} from './usage.js'; /** `none`: an additive finding, nothing to change; it counts in callSitesChecked only. */ export type Fixability = 'mechanical' | 'assisted' | 'manual' | 'unknown' | 'none'; @@ -30,6 +36,18 @@ export interface Finding { * one site to fix, and `downstream` is the evidence; it counts as one site, not as many. */ anchorOnly?: true; + /** + * The repository parameter this finding's compile error descends from, when it stands alone in + * its workspace: merged across workspaces, sites that trip the same declaration become one + * `cause` anchor at it. Dropped once joined or when no other site shares it. + */ + sharedCause?: DiagnosticCause; + /** + * For a `cause` anchor at a repository parameter: how many call sites trip it, across every + * workspace (`downstream` lists them). Absent for a compiler-option anchor, whose errors are + * not call sites. + */ + callSites?: number; /** * For a `module-format` finding: every `require()` site of the package in this file, the * finding's own first. Switching a file to `import()` is one unit of work however many diff --git a/packages/core/src/domain/usage.ts b/packages/core/src/domain/usage.ts index b7030f48..1de45937 100644 --- a/packages/core/src/domain/usage.ts +++ b/packages/core/src/domain/usage.ts @@ -109,6 +109,12 @@ export interface CompileDiagnostic { snippet: string; /** The repo declaration this error descends from, when one could be traced (see adapters/typescript/cause.ts). */ cause?: DiagnosticCause; + /** + * The repository parameter this error descends from when only this site in its workspace trips + * it: not a cause yet (one site keeps its own diagnostic), but a candidate to join the sites of + * the other workspaces that trip the same declaration (`check/shared-root.ts`). + */ + sharedCause?: DiagnosticCause; } export interface DiagnosticCause { diff --git a/packages/core/src/packs/tooling/pack-test.test.ts b/packages/core/src/packs/tooling/pack-test.test.ts index 55981964..69f62f3d 100644 --- a/packages/core/src/packs/tooling/pack-test.test.ts +++ b/packages/core/src/packs/tooling/pack-test.test.ts @@ -1,10 +1,12 @@ import { readFileSync } from 'node:fs'; import { fileURLToPath } from 'node:url'; import { expect, it } from 'vitest'; +import type { CheckReport } from '../../domain/report.js'; import { aiPack } from '../ai/index.js'; import type { PackVerification } from '../contract.js'; +import { definePack } from '../contract.js'; import { registeredPacks } from '../index.js'; -import { testPack } from './pack-test.js'; +import { predictedSites, scoreSites, testPack } from './pack-test.js'; const root = fileURLToPath(new URL('../../../../../', import.meta.url)); const verification = JSON.parse( @@ -26,3 +28,49 @@ it('passes every registered pack: each companion it names carries its source', a expect(report.problems, entry.pack.name).toEqual([]); } }); + +it('scores an anchored finding at its declaration; the call sites under it are neither true nor false positives', () => { + const pack = definePack({ + meta: { + package: 'react', + from: '>=18 <19', + to: '>=19 <20', + sources: [{ title: 'guide', url: 'https://example.com' }], + maintainer: 'uptide-dev', + }, + rules: [], + instructions: '', + }); + const decl = { file: 'packages/editor/src/lib/hooks/useTransform.ts', line: 6 }; + const call = { file: 'apps/examples/src/x.tsx', line: 11 }; + const report = { + packages: [ + { + workspace: '*', + name: 'react', + findings: [ + { + change: { path: 'cause:ref', kind: 'cause' }, + usage: decl, + anchorOnly: true, + callSites: 2, + downstream: [ + { ...call, code: 2345, message: 'x' }, + { file: 'packages/tldraw/src/y.tsx', line: 59, code: 2345, message: 'x' }, + ], + }, + ], + // A call site that is also planned on its own (say, by a rule's detect) is still evidence. + plan: [ + { rule: 'cause:ref', severity: 'breaking', locations: [decl] }, + { rule: 'TS2345', severity: 'breaking', locations: [call] }, + ], + }, + ], + } as unknown as CheckReport; + const predicted = predictedSites(report, pack); + expect(predicted.map((s) => `${s.file}:${s.line}`)).toEqual([`${decl.file}:${decl.line}`]); + const score = scoreSites(predicted, [{ ...decl, rule: 'generic' }]); + expect(score.falsePositives).toEqual([]); + expect(score.falseNegatives).toEqual([]); +}); diff --git a/packages/core/src/packs/tooling/pack-test.ts b/packages/core/src/packs/tooling/pack-test.ts index 345d87d4..36646bc0 100644 --- a/packages/core/src/packs/tooling/pack-test.ts +++ b/packages/core/src/packs/tooling/pack-test.ts @@ -103,12 +103,36 @@ function tally(tp: number, fp: number, fn: number): Tally { const key = (s: { file: string; line: number }): string => `${s.file}:${s.line}`; +/** + * The call sites listed under a one-edit anchor (a repository parameter, a compiler option): + * evidence of the anchor, never sites of their own. Repository-relative. + */ +export function evidenceSites(report: CheckReport, pack: Pack): Set { + const evidence = new Set(); + for (const p of report.packages) { + const names = p.members ? p.members.map((m) => m.name) : [p.name]; + if (!names.includes(pack.name)) continue; + const prefix = p.workspace === '.' || p.workspace === '*' ? '' : `${p.workspace}/`; + for (const f of p.findings) { + if (f.change.kind !== 'cause' || !f.anchorOnly) continue; + for (const d of f.downstream ?? []) + evidence.add( + key({ file: d.file.startsWith(prefix) ? d.file : `${prefix}${d.file}`, line: d.line }), + ); + } + } + return evidence; +} + /** * The sites `check` shows for the package, from its plan: the findings worth acting on, * grouped under the rule `fix` would apply. Repository-relative, as ground truth is written. + * An anchored finding is scored at its anchor (the declaration a ground-truth finding names); + * the call sites under it are evidence, neither true nor false positives. */ export function predictedSites(report: CheckReport, pack: Pack): ScoredSite[] { const own = new Set([...pack.rules.map((r) => r.id), ...pack.behavior.map((b) => b.id)]); + const evidence = evidenceSites(report, pack); const sites = new Map(); for (const p of report.packages) { const names = p.members ? p.members.map((m) => m.name) : [p.name]; @@ -117,6 +141,7 @@ export function predictedSites(report: CheckReport, pack: Pack): ScoredSite[] { for (const group of p.plan ?? []) for (const location of group.locations) { const file = location.file.startsWith(prefix) ? location.file : `${prefix}${location.file}`; + if (evidence.has(key({ file, line: location.line }))) continue; const site: ScoredSite = { file, line: location.line,