From aff09e1f77f7fff4d2661e49e340d39e35a59d04 Mon Sep 17 00:00:00 2001 From: Lucas Braum Date: Fri, 9 Oct 2026 15:45:21 +0000 Subject: [PATCH 1/2] feat(check): one finding when call sites in several workspaces trace to one declaration A mismatch at an argument traces to the repository parameter it lands on (parameterCause); the compile signal anchored the finding there only when two or more sites in the same workspace tripped it. tldraw's useTransform(ref) is called from three workspaces, each with one site, so check reported three findings beside the one at the hook's parameter that upstream actually edited. - The diagnostic keeps the declaration it traces to (`root`), and so does the call-site finding (`Finding.root`, repository-relative), whether or not its workspace folds it. - After the workspaces are merged, foldSharedRoots (check/root-cause.ts) compares roots across them: two or more sites at one declaration, or a site plus the anchor another workspace already produced, become one anchored finding at the declaration, with the sites as `downstream` (each with its `workspace`) and `details` opening with "N call sites in M workspaces"; the pack rule the sites agreed on is kept. - The terminal (--details) prints that line with a few sites as examples; the HTML report lists the call sites with their workspaces under the anchor; the JSON report only gains fields (`root`, `downstream[].workspace`). - The pack test scorer counts the anchor as the predicted site and never the evidence under it, even when another finding lands on an evidence line. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_018u9b3Pp5H68DdYZGp5Qg1U Signed-off-by: Lucas Braum --- .changeset/check-shared-roots.md | 13 ++ docs/architecture.md | 15 +- docs/cli.md | 8 +- packages/cli/src/format-check-details.test.ts | 59 ++++++++ packages/cli/src/format-check-details.ts | 10 +- packages/cli/src/html/render.test.ts | 62 ++++++++ packages/cli/src/html/render.ts | 41 +++++- .../src/adapters/typescript/cause.test.ts | 10 ++ .../core/src/adapters/typescript/compile.ts | 4 + packages/core/src/check/check.test.ts | 74 ++++++++++ packages/core/src/check/check.ts | 34 +++-- packages/core/src/check/root-cause.test.ts | 139 +++++++++++++++++- packages/core/src/check/root-cause.ts | 98 +++++++++++- packages/core/src/check/unattributed.ts | 17 ++- packages/core/src/domain/report.ts | 19 +++ packages/core/src/domain/usage.ts | 6 + packages/core/src/index.ts | 1 + .../core/src/packs/tooling/pack-test.test.ts | 94 +++++++++++- packages/core/src/packs/tooling/pack-test.ts | 25 +++- 19 files changed, 704 insertions(+), 25 deletions(-) create mode 100644 .changeset/check-shared-roots.md diff --git a/.changeset/check-shared-roots.md b/.changeset/check-shared-roots.md new file mode 100644 index 0000000..1d35b60 --- /dev/null +++ b/.changeset/check-shared-roots.md @@ -0,0 +1,13 @@ +--- +"uptide": minor +--- + +`check` reports one breaking finding when call sites in different workspaces trace back to +the same local declaration (a parameter or prop whose type names something from the upgraded +package): the finding is anchored at the declaration, where the one edit is, and the call +sites are listed under it as evidence with an `N call sites in M workspaces` line, in the +terminal, the HTML report and the JSON report. Before, a site alone in its workspace stayed a +finding of its own, so a hook typed `RefObject` and called from three +workspaces was three findings plus the one that mattered. The JSON report gains `root` on +call-site findings and `workspace` on evidence sites; nothing is renamed or removed. The pack +test scorer counts the anchor as the predicted site and never the evidence under it. diff --git a/docs/architecture.md b/docs/architecture.md index c7b8689..c3ad001 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -589,8 +589,19 @@ 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 its workspace is left to that +site, which may as well be the argument's, but the declaration it traces to is kept on the +diagnostic (`root`) and on the finding (`Finding.root`, repository-relative). After the +workspaces are merged, `foldSharedRoots` (`check/root-cause.ts`) compares roots across them: +two or more sites at one declaration, or a site plus the anchor another workspace already +produced, become one anchored finding at the declaration (`anchorOnly`, the sites as +`downstream`, each with its `workspace`, `details` opening with `N call sites in M +workspaces`), filed under the pack rule the sites agreed on. tldraw's `useTransform(ref)` is +called from three workspaces and was fixed upstream once, in the hook's parameter; `check` +now reports that one site. The JSON report only gains fields for this (`root`, +`downstream[].workspace`); nothing is renamed or removed. The pack test scorer counts the +anchor as the predicted site and never the evidence under it, even when another finding +lands on an evidence line. 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/docs/cli.md b/docs/cli.md index afc74e9..ff53dfc 100644 --- a/docs/cli.md +++ b/docs/cli.md @@ -126,7 +126,13 @@ from it, every element in every file errors with one fix: `check` reports one fi diagnostics as evidence, and `--details` shows a few of them. A repository parameter that several call sites trip over (a hook typed `RefObject` once `useRef` returns `RefObject`) is reported the same way: one finding at the parameter, with -the call sites as evidence. +the call sites as evidence, and `--details` prints `N call sites in M workspaces` under it +with a few of them. The call sites may sit in other workspaces than the parameter (a hook in +`packages/editor` called from `apps/examples` and `packages/tldraw`): a site alone in its +workspace still folds into the one finding at the declaration once the workspaces are merged. +The HTML report lists the call sites, with their workspaces, under the anchor; the JSON report +carries them as the finding's `downstream`, and `root` on each call-site finding names the +declaration it traces to. ### Partial results diff --git a/packages/cli/src/format-check-details.test.ts b/packages/cli/src/format-check-details.test.ts index 823b307..8d886a6 100644 --- a/packages/cli/src/format-check-details.test.ts +++ b/packages/cli/src/format-check-details.test.ts @@ -450,3 +450,62 @@ it('prints one native warning for a package regardless of how many findings it c const text = formatCheck({ ...report, packages: [p] }); expect(text.match(/native package, runtime probe skipped/g)).toHaveLength(1); }); + +it('prints the call sites under a declaration several workspaces trip, then a few as examples', () => { + const root = { + name: 'ref', + file: 'packages/editor/src/useTransform.ts', + line: 6, + reason: + "whose parameter `ref: RefObject` no longer accepts what the target gives it; widen the parameter's type there", + }; + const anchor = finding({ + change: { package: 'react', from: '18.3.1', to: '19.2.1', kind: 'cause', path: 'cause:ref' }, + usage: { file: root.file, line: 6, symbolPath: 'cause:ref', access: 'read', via: 'inferred' }, + anchorOnly: true, + root, + downstream: [ + { + file: 'apps/examples/src/Custom.tsx', + line: 11, + code: 2345, + message: 'not assignable', + workspace: 'apps/examples', + }, + { + file: 'packages/tldraw/src/Foreground.tsx', + line: 59, + code: 2345, + message: 'not assignable', + workspace: 'packages/tldraw', + }, + ], + details: ['2 call sites in 2 workspaces'], + fixability: 'assisted', + reason: root.reason, + evidence: 'compiler', + }); + const out = formatCheck( + { + ...report, + packages: [ + { + ...(report.packages[0] as PackageReport), + name: 'react', + workspace: '*', + installed: '18.3.1', + target: '19.2.1', + latest: '19.2.1', + findings: [anchor], + }, + ], + }, + { color: false }, + ); + expect(out).toContain(' ✗ packages/editor/src/useTransform.ts:6 `ref`'); + expect(out).toContain(' 2 call sites in 2 workspaces, for example'); + expect(out).toContain(' apps/examples/src/Custom.tsx:11 TS2345: not assignable'); + expect(out).toContain(' packages/tldraw/src/Foreground.tsx:59 TS2345: not assignable'); + // The count is printed once, not again from the details. + expect(out.split('2 call sites in 2 workspaces')).toHaveLength(2); +}); diff --git a/packages/cli/src/format-check-details.ts b/packages/cli/src/format-check-details.ts index e90e2d9..2efbefb 100644 --- a/packages/cli/src/format-check-details.ts +++ b/packages/cli/src/format-check-details.ts @@ -1,5 +1,6 @@ import { type CheckReport, + callSitesLine, type Finding, isNativeProbeSkip, type PackageReport, @@ -99,7 +100,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.root + ? ` ${callSitesLine(sites)}${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}`), @@ -107,7 +110,10 @@ function siteLines( ...(sites.length > shown.length ? [colors.dim(` … and ${sites.length - shown.length} more`)] : []), - ...(f.details ?? []).map((detail) => colors.dim(` ${detail}`)), + // The call-site count is printed above; the rest of the details follow. + ...(f.details ?? []) + .filter((detail) => !(f.root && /^\d+ call sites?\b/.test(detail))) + .map((detail) => colors.dim(` ${detail}`)), ]; } return [ diff --git a/packages/cli/src/html/render.test.ts b/packages/cli/src/html/render.test.ts index 6fb4c5f..a9ff2c3 100644 --- a/packages/cli/src/html/render.test.ts +++ b/packages/cli/src/html/render.test.ts @@ -183,3 +183,65 @@ it('omits workspace paths from check rows by default and counts breaking fixes c expect(html).toContain('25Auto-fixable'); expect(html).toContain('7Need the agent (LLM)'); }); + +it('lists the call sites under a declaration anchor, with their workspaces', () => { + const report = fixture('stripe'); + const p = report.packages.find( + (pkg) => pkg.findings.length > 0, + ) as (typeof report.packages)[number]; + const first = p.findings[0] as NonNullable<(typeof p.findings)[number]>; + const root = { + name: 'ref', + file: 'packages/editor/src/useTransform.ts', + line: 6, + reason: 'whose parameter changed', + }; + p.findings = [ + { + ...first, + change: { ...first.change, kind: 'cause', path: 'cause:ref' }, + usage: { ...first.usage, file: root.file, line: 6, compileError: undefined }, + anchorOnly: true, + root, + severity: 'breaking', + reason: root.reason, + downstream: [ + { + file: 'apps/examples/src/Custom.tsx', + line: 11, + code: 2345, + message: 'm', + workspace: 'apps/examples', + }, + { + file: 'packages/tldraw/src/Foreground.tsx', + line: 59, + code: 2345, + message: 'm', + workspace: 'packages/tldraw', + }, + ], + details: ['2 call sites in 2 workspaces'], + }, + ]; + p.plan = [ + { + rule: 'TS2345', + title: 'ref', + severity: 'breaking', + by: { rule: 0, agent: 1, manual: 0 }, + sites: 1, + fixes: 1, + locations: [{ file: root.file, line: 6 }], + detail: root.reason, + }, + ]; + const html = renderHtml(report, opts); + expect(html).toContain('2 call sites in 2 workspaces'); + expect(html).toContain( + '
  • apps/examples/src/Custom.tsx:11 apps/examples
    • ', + ); + expect(html).toContain( + '
  • packages/tldraw/src/Foreground.tsx:59 packages/tldraw
    • ', + ); +}); diff --git a/packages/cli/src/html/render.ts b/packages/cli/src/html/render.ts index ef59dc7..d540978 100644 --- a/packages/cli/src/html/render.ts +++ b/packages/cli/src/html/render.ts @@ -1,5 +1,12 @@ import { basename } from 'node:path'; -import { type CheckReport, isFailure, type PackageReport, type PlanGroup } from '@uptide/core'; +import { + type CheckReport, + callSitesLine, + type Finding, + isFailure, + type PackageReport, + type PlanGroup, +} from '@uptide/core'; import { byLine, checkRows, type FormatCheckOptions, nextCommands } from '../format-check.js'; import { groupReason, groupsForHtml, notesOf, stripeNote, verdict } from './content.js'; import { escapeHtml } from './escape.js'; @@ -47,6 +54,23 @@ function compilerAt(p: PackageReport, group: PlanGroup, file: string, line: numb ].join(' · ') || (group.severity !== 'deprecated' ? group.detail : '') ); } +/** The anchored finding at a site, when the site is the one edit several call sites trace to. */ +function anchorAt( + p: PackageReport, + group: PlanGroup, + file: string, + line: number, +): Finding | undefined { + return p.findings.find( + (f) => + f.anchorOnly && + f.root !== undefined && + f.severity === group.severity && + f.usage.file === file && + f.usage.line === line && + (f.downstream?.length ?? 0) > 0, + ); +} /** 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,20 @@ export function renderHtml(report: CheckReport, opts: HtmlOptions): string { ? `
    compiler messageSame message, shown above
    ` : `
    compiler message
    ${escapeHtml(message)}
    `; } - const article = `
    ${link}
    ${excerpt}${compiler}
    `; + // One edit at the anchor resolves every call site listed under it. + const anchor = anchorAt(p, g, site.file, site.line); + const callSites = anchor?.downstream + ? `
    ${e(callSitesLine(anchor.downstream))}
      ${anchor.downstream + .slice(0, 50) + .map( + (d) => + `
    • ${e(`${d.file}:${d.line}`)}${d.workspace && d.workspace !== '.' ? ` ${e(d.workspace)}` : ''}
      • `, + ) + .join( + '', + )}${anchor.downstream.length > 50 ? `
    • and ${anchor.downstream.length - 50} more
      • ` : ''}
    ` + : ''; + const article = `
    ${link}
    ${excerpt}${compiler}${callSites}
    `; 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 84e74e8..73394a9 100644 --- a/packages/core/src/adapters/typescript/cause.test.ts +++ b/packages/core/src/adapters/typescript/cause.test.ts @@ -126,5 +126,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(); + // The declaration it traces to is kept: a site in another workspace at the same + // parameter folds with it after the merge (check/root-cause.ts). + expect(at5?.root).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 0680b3e..8f078d9 100644 --- a/packages/core/src/adapters/typescript/compile.ts +++ b/packages/core/src/adapters/typescript/compile.ts @@ -924,6 +924,10 @@ export async function compileAgainstTargets( : undefined) ?? findCause(programs, d, overlaid, repo.dir, erroredLines); if (cause) diagnostic.cause = cause; + // Traced to a parameter but alone in this workspace: the root is kept, so another + // workspace's site at the same declaration can fold with it after the merge. + const root = parameter !== undefined ? byParameter.get(parameter) : undefined; + if (root) diagnostic.root = root; return diagnostic; }); return { diff --git a/packages/core/src/check/check.test.ts b/packages/core/src/check/check.test.ts index 67b8dc4..2ccb6a2 100644 --- a/packages/core/src/check/check.test.ts +++ b/packages/core/src/check/check.test.ts @@ -438,6 +438,80 @@ describe('mergeAcrossWorkspaces', () => { }); }); + it('folds sites in two workspaces that trace to one declaration, prefixing the root by workspace', () => { + const root = (file: string) => ({ + name: 'ref', + file, + line: 6, + reason: 'whose parameter changed', + }); + const site = (file: string, line: number, rootFile: string): Finding => ({ + change: { + package: 'react', + from: '18.3.1', + to: '19.2.1', + path: 'TS2345', + kind: 'type', + severity: 'breaking', + source: 'types', + confidence: 1, + }, + usage: { + file, + line, + column: 1, + endLine: line, + endColumn: 2, + symbolPath: 'TS2345', + access: 'read', + via: 'inferred', + snippet: '', + compileCode: 2345, + compileError: 'mismatch', + }, + root: root(rootFile), + severity: 'breaking', + confidence: 1, + fixability: 'unknown', + reason: 'compile error', + evidence: 'compiler', + }); + const out = mergeAcrossWorkspaces( + [ + base('apps/examples', 'react', { + installed: '18.3.1', + findings: [site('src/Custom.tsx', 11, '../../packages/editor/src/useTransform.ts')], + }), + base('packages/tldraw', 'react', { + installed: '18.3.1', + findings: [site('src/Foreground.tsx', 59, '../editor/src/useTransform.ts')], + }), + ], + {}, + ); + expect(out).toHaveLength(1); + expect(out[0]?.findings).toHaveLength(1); + expect(out[0]?.findings[0]).toMatchObject({ + anchorOnly: true, + usage: { file: 'packages/editor/src/useTransform.ts', line: 6 }, + root: root('packages/editor/src/useTransform.ts'), + details: ['2 call sites in 2 workspaces'], + }); + expect(out[0]?.findings[0]?.downstream).toEqual([ + expect.objectContaining({ + file: 'apps/examples/src/Custom.tsx', + line: 11, + workspace: 'apps/examples', + }), + expect.objectContaining({ + file: 'packages/tldraw/src/Foreground.tsx', + line: 59, + workspace: 'packages/tldraw', + }), + ]); + expect(out[0]?.status).toBe('breaking'); + }); + it('merges coverage across workspaces, naming each compiler once', () => { const compile = (version: string, own: boolean) => ({ baselineErrors: 0, diff --git a/packages/core/src/check/check.ts b/packages/core/src/check/check.ts index 083f627..34543d3 100644 --- a/packages/core/src/check/check.ts +++ b/packages/core/src/check/check.ts @@ -52,7 +52,7 @@ import { } from './module-format.js'; import { mapWithLimit, mapWithSerialRetry } from './pool.js'; import { isBehind, rankCandidates } from './rank.js'; -import { groupRootCauses } from './root-cause.js'; +import { foldSharedRoots, groupRootCauses } from './root-cause.js'; import { runtimeChangeFindings } from './runtime-changes.js'; import { confirmBreaking, evidenceOf, tierOf } from './tier.js'; import { typesPackageOf, typesReleaseFor } from './types-release.js'; @@ -563,15 +563,16 @@ export function mergeAcrossWorkspaces( const first = list[0] as PackageReport; const sum = (pick: (p: PackageReport) => number): number => list.reduce((n, p) => n + pick(p), 0); - const combined: PackageReport = { - ...first, - workspace: '*', - workspaces: list.map((p) => p.workspace), - ...(catalog ? { source: 'catalog' as const } : {}), - findings: list.flatMap((p) => - p.findings.map((f) => ({ + const workspaceOf = new WeakMap(); + const findings = list.flatMap((p) => + p.findings.map((f) => { + const moved: Finding = { ...f, usage: { ...f.usage, file: prefixed(p, f.usage.file) }, + ...(f.root ? { root: { ...f.root, file: prefixed(p, f.root.file) } } : {}), + ...(f.downstream + ? { downstream: f.downstream.map((d) => ({ workspace: p.workspace, ...d })) } + : {}), // A cluster's description names its cause by file: the same prefix applies there. change: f.change.path.startsWith('cause:') ? { @@ -582,8 +583,19 @@ export function mergeAcrossWorkspaces( ), } : f.change, - })), - ), + }; + workspaceOf.set(moved, p.workspace); + return moved; + }), + ); + // Sites in several workspaces that trace to one declaration: one finding, at the declaration. + const folded = foldSharedRoots(findings, (f) => workspaceOf.get(f)); + const combined: PackageReport = { + ...first, + workspace: '*', + workspaces: list.map((p) => p.workspace), + ...(catalog ? { source: 'catalog' as const } : {}), + findings: folded, callSitesChecked: sum((p) => p.callSitesChecked), unanalyzed: list.flatMap((p) => p.unanalyzed.map((u) => ({ ...u, file: prefixed(p, u.file) })), @@ -593,7 +605,7 @@ export function mergeAcrossWorkspaces( ? { skipReason: list.find((p) => p.skipReason)?.skipReason } : {}), status: statusOf( - list.flatMap((p) => p.findings), + folded, sum((p) => p.callSitesChecked), sum((p) => p.unanalyzed.length), ), diff --git a/packages/core/src/check/root-cause.test.ts b/packages/core/src/check/root-cause.test.ts index 074bbdc..7e30edf 100644 --- a/packages/core/src/check/root-cause.test.ts +++ b/packages/core/src/check/root-cause.test.ts @@ -1,8 +1,8 @@ -import { expect, it } from 'vitest'; +import { describe, expect, it } from 'vitest'; import type { Finding, PackageReport } from '../domain/report.js'; import { planPackage } from '../fix/plan.js'; import { sitesOf } from './check.js'; -import { groupRootCauses } from './root-cause.js'; +import { callSitesLine, foldSharedRoots, groupRootCauses } from './root-cause.js'; it('groups missing TypeScript API members without losing sites or unrelated diagnostics', () => { const finding = (line: number, code = 2339): Finding => ({ @@ -64,3 +64,138 @@ it('groups missing TypeScript API members without losing sites or unrelated diag groupRootCauses(withoutEvidence); expect(withoutEvidence.findings[0]?.change.kind).toBe('removed'); }); + +describe('foldSharedRoots', () => { + const root = { + name: 'ref', + file: 'packages/editor/src/useTransform.ts', + line: 6, + reason: + "whose parameter `ref: RefObject` no longer accepts what the target gives it; widen the parameter's type there", + }; + const site = (file: string, line: number, over: Partial = {}): 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: 16, + endLine: line, + endColumn: 20, + symbolPath: 'TS2345', + access: 'read', + via: 'inferred', + snippet: 'useTransform(rSvg, x, y)', + compileCode: 2345, + compileError: "Argument of type 'RefObject' is not assignable", + }, + root, + severity: 'breaking', + confidence: 1, + fixability: 'unknown', + reason: 'compile error not attributed to a known API change', + evidence: 'compiler', + rule: 'ref-object-nullable', + ...over, + }); + const workspaces = new Map(); + const at = (workspace: string, f: Finding): Finding => { + workspaces.set(f, workspace); + return f; + }; + + it('folds sites in different workspaces that trace to one declaration into one anchored finding', () => { + const a = at('apps/examples', site('apps/examples/src/Custom.tsx', 11)); + const b = at('packages/tldraw', site('packages/tldraw/src/Foreground.tsx', 59)); + const other = at('apps/examples', site('apps/examples/src/Other.tsx', 3, { root: undefined })); + const out = foldSharedRoots([a, other, b], (f) => workspaces.get(f)); + expect(out).toHaveLength(2); + const anchor = out[0] as Finding; + expect(anchor).toMatchObject({ + change: { kind: 'cause', path: 'cause:ref', severity: 'breaking' }, + usage: { file: root.file, line: 6, symbolPath: 'cause:ref' }, + anchorOnly: true, + root, + severity: 'breaking', + fixability: 'assisted', + reason: root.reason, + rule: 'ref-object-nullable', + evidence: 'compiler', + details: ['2 call sites in 2 workspaces'], + }); + expect(anchor.change.notes).toBe( + `2 errors caused by \`ref\` in ${root.file}:6: ${root.reason}. One edit there resolves them.`, + ); + expect(anchor.downstream).toEqual([ + { + file: 'apps/examples/src/Custom.tsx', + line: 11, + column: 16, + snippet: 'useTransform(rSvg, x, y)', + code: 2345, + message: "Argument of type 'RefObject' is not assignable", + workspace: 'apps/examples', + }, + expect.objectContaining({ + file: 'packages/tldraw/src/Foreground.tsx', + line: 59, + workspace: 'packages/tldraw', + }), + ]); + expect(sitesOf(anchor)).toBe(1); + expect(out[1]).toBe(other); + }); + + it('leaves a site alone at its root, and joins a site to the anchor another workspace already produced', () => { + const alone = site('apps/examples/src/Custom.tsx', 11); + expect(foldSharedRoots([alone], () => 'apps/examples')).toEqual([alone]); + const anchor: Finding = { + ...site(root.file, 6, { fixability: 'assisted', reason: root.reason }), + change: { ...site('', 0).change, kind: 'cause', path: 'cause:ref' }, + anchorOnly: true, + downstream: [ + { + file: 'packages/editor/src/A.tsx', + line: 4, + code: 2345, + message: 'm', + workspace: 'packages/editor', + }, + { + file: 'packages/editor/src/B.tsx', + line: 9, + code: 2345, + message: 'm', + workspace: 'packages/editor', + }, + ], + }; + const out = foldSharedRoots([anchor, at('apps/examples', alone)], (f) => workspaces.get(f)); + expect(out).toHaveLength(1); + expect(out[0]?.downstream?.map((d) => `${d.file}:${d.line}`)).toEqual([ + 'apps/examples/src/Custom.tsx:11', + 'packages/editor/src/A.tsx:4', + 'packages/editor/src/B.tsx:9', + ]); + expect(out[0]?.details).toEqual(['3 call sites in 2 workspaces']); + }); + + it('counts workspaces only when the sites span more than one', () => { + expect(callSitesLine([{ file: 'a', line: 1, code: 1, message: '' }])).toBe('1 call site'); + expect( + callSitesLine([ + { file: 'a', line: 1, code: 1, message: '', workspace: 'x' }, + { file: 'b', line: 1, code: 1, message: '', workspace: 'x' }, + ]), + ).toBe('2 call sites'); + }); +}); diff --git a/packages/core/src/check/root-cause.ts b/packages/core/src/check/root-cause.ts index 5dadfee..db4a867 100644 --- a/packages/core/src/check/root-cause.ts +++ b/packages/core/src/check/root-cause.ts @@ -1,6 +1,102 @@ -import type { Finding, PackageReport } from '../domain/report.js'; +import type { DownstreamSite, Finding, PackageReport, SharedRoot } from '../domain/report.js'; +import { describeCluster } from './unattributed.js'; import { parseVersion } from './version.js'; +const rootKey = (r: SharedRoot): string => `${r.file}:${r.line}:${r.name}`; + +/** `3 call sites in 2 workspaces`: the line under a shared root's reason. */ +export function callSitesLine(sites: readonly DownstreamSite[]): string { + const workspaces = new Set(sites.map((s) => s.workspace).filter((w) => w !== undefined)); + const count = (n: number, noun: string): string => `${n} ${noun}${n === 1 ? '' : 's'}`; + return `${count(sites.length, 'call site')}${workspaces.size > 1 ? ` in ${count(workspaces.size, 'workspace')}` : ''}`; +} + +/** + * Sites in different workspaces that trace to one repository declaration are one finding, + * anchored at the declaration with the sites as evidence. Within a workspace the compile + * signal already folds two or more such sites (adapters/typescript/compile.ts); a site alone + * in its workspace keeps its `root`, and here, after the merge, roots are compared across + * workspaces: two or more sites at one root, or one site plus the anchor another workspace + * already produced, become (or join) the anchored finding. tldraw's `useTransform(ref)`, + * called from three workspaces, is fixed once, in the hook's parameter. + */ +export function foldSharedRoots( + findings: Finding[], + workspaceOf: (f: Finding) => string | undefined, +): Finding[] { + const groups = new Map(); + for (const f of findings) { + if (!f.root) continue; + const list = groups.get(rootKey(f.root)) ?? []; + list.push(f); + groups.set(rootKey(f.root), list); + } + const folded = new Map(); + for (const group of groups.values()) { + const anchors = group.filter((f) => f.anchorOnly && f.change.kind === 'cause'); + const sites = group.filter((f) => !anchors.includes(f)); + // One site, no anchor anywhere: nothing shared, the site stands on its own. + if (sites.length === 0 || anchors.length + sites.length < 2) continue; + const root = (group[0] as Finding).root as SharedRoot; + const evidence: DownstreamSite[] = [ + ...anchors.flatMap((a) => a.downstream ?? []), + ...sites.map((f) => ({ + file: f.usage.file, + line: f.usage.line, + column: f.usage.column, + snippet: f.usage.snippet, + code: f.usage.compileCode ?? Number(/^TS(\d+)$/.exec(f.change.path)?.[1] ?? 0), + message: f.usage.compileError ?? f.reason, + ...(workspaceOf(f) !== undefined ? { workspace: workspaceOf(f) } : {}), + })), + ].sort((a, b) => a.file.localeCompare(b.file) || a.line - b.line); + const first = (anchors[0] ?? sites[0]) as Finding; + // The pack rule the sites were filed under, when they agree on one. + const rules = [...new Set(group.map((f) => f.rule).filter((r) => r !== undefined))]; + const anchor: Finding = { + ...first, + change: { + ...first.change, + path: `cause:${root.name}`, + kind: 'cause', + severity: 'breaking', + evidence: 'checker', + notes: describeCluster(evidence.length, { ...root, anchorOnly: true }), + }, + usage: { + file: root.file, + line: root.line, + column: 1, + endLine: root.line, + endColumn: 1, + symbolPath: `cause:${root.name}`, + access: 'read', + snippet: '', + via: 'inferred', + }, + downstream: evidence, + anchorOnly: true, + root, + severity: 'breaking', + confidence: 1, + fixability: 'assisted', + reason: root.reason, + details: [ + callSitesLine(evidence), + ...(first.details ?? []).filter((d) => !/^\d+ call sites?/.test(d)), + ], + evidence: 'compiler', + ...(rules.length === 1 ? { rule: rules[0] } : {}), + }; + folded.set(first, anchor); + for (const f of group) if (f !== first) folded.set(f, undefined); + } + if (folded.size === 0) return findings; + return findings.flatMap((f) => + folded.has(f) ? (folded.get(f) ? [folded.get(f) as Finding] : []) : [f], + ); +} + /** Known package-level cause, gated on compiler evidence. Other diagnostics remain separate. */ export function groupRootCauses(report: PackageReport): void { if (report.name !== 'typescript' || parseVersion(report.target)?.major !== 7) return; diff --git a/packages/core/src/check/unattributed.ts b/packages/core/src/check/unattributed.ts index 3b660bc..1b2eeb7 100644 --- a/packages/core/src/check/unattributed.ts +++ b/packages/core/src/check/unattributed.ts @@ -1,6 +1,6 @@ import { posix } from 'node:path'; import type { Change } from '../domain/change.js'; -import type { Finding } from '../domain/report.js'; +import type { Finding, SharedRoot } from '../domain/report.js'; import type { ApiSurface } from '../domain/surface.js'; import type { CompileDiagnostic, DiagnosticCause, Usage } from '../domain/usage.js'; @@ -93,13 +93,24 @@ export function unattributedFindings( // 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 } : {}), + ...(cause.anchorOnly && !cause.config ? { root: sharedRoot(cause, filePrefix) } : {}), severity: 'breaking', confidence: 1, fixability: cause.anchorOnly || cause.config ? 'assisted' : 'unknown', reason: cause.anchorOnly || cause.config ? cause.reason : UNATTRIBUTED_REASON, }; }); - return [...clustered, ...plainFindings(single, meta, surfaceB, unresolvedFiles)]; + return [...clustered, ...plainFindings(single, meta, surfaceB, unresolvedFiles, filePrefix)]; +} + +/** The declaration as the repository names it: a `../editor/x.ts` seen from a workspace is `packages/editor/x.ts`. */ +function sharedRoot(cause: DiagnosticCause, filePrefix: string): SharedRoot { + return { + name: cause.name, + file: posix.normalize(`${filePrefix}${cause.file}`), + line: cause.line, + reason: cause.reason, + }; } function plainFindings( @@ -107,6 +118,7 @@ function plainFindings( meta: { package: string; from: string; to: string }, surfaceB: ApiSurface, unresolvedFiles: string[], + filePrefix: string, ): Finding[] { const unresolved = new Set(unresolvedFiles); // Leaf names declared in files the compiler could not fully resolve. @@ -142,6 +154,7 @@ function plainFindings( return { change, usage, + ...(d.root ? { root: sharedRoot(d.root, filePrefix) } : {}), 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 23de32f..0e8f389 100644 --- a/packages/core/src/domain/report.ts +++ b/packages/core/src/domain/report.ts @@ -14,12 +14,24 @@ export interface DownstreamSite { line: number; code: number; message: string; + /** The workspace the site is in, when the anchor gathers sites from several. */ + workspace?: string; } /** `verified`: a migration pack covers the upgrade. `generic`: analysis only, no pack. */ export type Tier = 'verified' | 'generic'; /** A change joined to one usage of the changed symbol, with severity resolved for that usage. */ +/** A local declaration several call sites trace back to: where one edit resolves them all. */ +export interface SharedRoot { + /** Declared name (`ref`). */ + name: string; + file: string; + line: number; + /** Why it is the edit, as a clause: "whose parameter `ref: RefObject` no longer accepts ...". */ + reason: string; +} + export interface Finding { change: Change; usage: Usage; @@ -30,6 +42,13 @@ export interface Finding { * one site to fix, and `downstream` is the evidence; it counts as one site, not as many. */ anchorOnly?: true; + /** + * The repository declaration (a parameter, a prop) a compile error at a call site traces + * to, repository-relative. On a call-site finding it says where the one edit is; sites in + * different workspaces that share it are folded into one anchored finding at the + * declaration (`anchorOnly`, the sites as `downstream`), which carries `root` too. + */ + root?: SharedRoot; /** * 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 b7030f4..0c3f7b2 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 or prop this mismatch traces to (`parameterCause`), set whether + * or not enough sites in this workspace share it for `cause` to be. Sites in other + * workspaces that trace to the same declaration fold into one finding at it (check/root-cause.ts). + */ + root?: DiagnosticCause; } export interface DiagnosticCause { diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index a09d1f9..81bd83a 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -18,6 +18,7 @@ export { resolveDirection } from './check/direction.js'; export { deprecationReplacement, fixabilityOf } from './check/fixability.js'; export { match } from './check/match.js'; export { type MergedSignals, mergeSignals, symbolFromMessage } from './check/merge.js'; +export { callSitesLine } from './check/root-cause.js'; export { TIER_LEGEND } from './check/tier.js'; export { formatTruthTable, diff --git a/packages/core/src/packs/tooling/pack-test.test.ts b/packages/core/src/packs/tooling/pack-test.test.ts index 5598196..b40a0d8 100644 --- a/packages/core/src/packs/tooling/pack-test.test.ts +++ b/packages/core/src/packs/tooling/pack-test.test.ts @@ -1,10 +1,11 @@ import { readFileSync } from 'node:fs'; import { fileURLToPath } from 'node:url'; import { expect, it } from 'vitest'; +import type { CheckReport, Finding } from '../../domain/report.js'; import { aiPack } from '../ai/index.js'; import type { PackVerification } from '../contract.js'; import { registeredPacks } from '../index.js'; -import { testPack } from './pack-test.js'; +import { predictedSites, testPack } from './pack-test.js'; const root = fileURLToPath(new URL('../../../../../', import.meta.url)); const verification = JSON.parse( @@ -26,3 +27,94 @@ 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 anchor; the call sites listed as evidence are not sites', () => { + const finding = (file: string, line: number, over: Partial = {}): Finding => ({ + change: { + package: 'ai', + from: '6.0.0', + to: '7.0.0', + path: 'TS2345', + kind: 'type', + severity: 'breaking', + source: 'types', + confidence: 1, + }, + usage: { + file, + line, + column: 1, + endLine: line, + endColumn: 2, + symbolPath: 'TS2345', + access: 'read', + via: 'inferred', + snippet: '', + }, + severity: 'breaking', + confidence: 1, + fixability: 'unknown', + reason: 'compile error', + ...over, + }); + const anchor = finding('packages/lib/hook.ts', 6, { + change: { ...finding('', 0).change, kind: 'cause', path: 'cause:ref' }, + anchorOnly: true, + downstream: [ + { file: 'apps/a/src/x.tsx', line: 11, code: 2345, message: 'm' }, + { file: 'apps/b/src/y.tsx', line: 59, code: 2345, message: 'm' }, + ], + }); + // A pack finding that happens to land on an evidence line is not scored either. + const onEvidence = finding('apps/a/src/x.tsx', 11, { rule: 'tool-context' }); + const report: CheckReport = { + repo: '/repo', + workspaces: ['apps/a', 'apps/b', 'packages/lib'], + packages: [ + { + workspace: '*', + name: 'ai', + installed: '6.0.0', + latest: '7.0.0', + target: '7.0.0', + majorsBehind: 1, + findings: [anchor, onEvidence], + callSitesChecked: 3, + unanalyzed: [], + status: 'breaking', + notes: [], + timing: { fetchMs: 0, diffMs: 0, usagesMs: 0, compileMs: 0 }, + plan: [ + { + rule: 'tool-context', + title: 't', + severity: 'breaking', + by: { rule: 0, agent: 2, manual: 0 }, + sites: 2, + fixes: 2, + locations: [ + { file: 'apps/a/src/x.tsx', line: 11 }, + { file: 'packages/lib/hook.ts', line: 6 }, + ], + detail: '', + }, + ], + }, + ], + summary: { + packagesNeedingAttention: 1, + breaking: 1, + deprecated: 0, + unverified: 0, + unaffected: 0, + notImported: 0, + partiallyAnalyzed: 0, + autoFixable: 0, + skippedForTime: 0, + failed: 0, + }, + }; + expect(predictedSites(report, aiPack).map((s) => `${s.file}:${s.line} ${s.rule}`)).toEqual([ + 'packages/lib/hook.ts:6 tool-context', + ]); +}); diff --git a/packages/core/src/packs/tooling/pack-test.ts b/packages/core/src/packs/tooling/pack-test.ts index 345d87d..04331de 100644 --- a/packages/core/src/packs/tooling/pack-test.ts +++ b/packages/core/src/packs/tooling/pack-test.ts @@ -114,9 +114,32 @@ export function predictedSites(report: CheckReport, pack: Pack): ScoredSite[] { 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}/`; + const absolute = (file: string): string => + file.startsWith(prefix) ? file : `${prefix}${file}`; + // A call site an anchored finding lists as evidence is neither a true nor a false + // positive: the one site is the anchor (the declaration, a compiler option), which the + // ground truth names, and the evidence under it is not scored even when another finding + // lands on the same line. + const evidence = new Set( + p.findings + .filter((f) => f.anchorOnly) + .flatMap((f) => + (f.downstream ?? []).map((d) => key({ file: absolute(d.file), line: d.line })), + ), + ); + const anchors = new Set( + p.findings + .filter((f) => f.anchorOnly) + .map((f) => key({ file: absolute(f.usage.file), line: f.usage.line })), + ); for (const group of p.plan ?? []) for (const location of group.locations) { - const file = location.file.startsWith(prefix) ? location.file : `${prefix}${location.file}`; + const file = absolute(location.file); + if ( + evidence.has(key({ file, line: location.line })) && + !anchors.has(key({ file, line: location.line })) + ) + continue; const site: ScoredSite = { file, line: location.line, From 43d3916c73ec8c91d766fed6c2616b8ae5c48c80 Mon Sep 17 00:00:00 2001 From: Lucas Braum Date: Fri, 9 Oct 2026 16:30:00 +0000 Subject: [PATCH 2/2] fix(check): keep a finding's root workspace-relative so the merge prefixes it once A root was prefixed by the workspace when the finding was made and again when the workspaces were merged (apps/examples/packages/editor/...), so roots never matched across workspaces. It is now kept like usage.file, and prefixed and normalized by the merge alone. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_018u9b3Pp5H68DdYZGp5Qg1U Signed-off-by: Lucas Braum --- packages/core/src/check/unattributed.ts | 22 ++++++++++------------ 1 file changed, 10 insertions(+), 12 deletions(-) diff --git a/packages/core/src/check/unattributed.ts b/packages/core/src/check/unattributed.ts index 1b2eeb7..4308f96 100644 --- a/packages/core/src/check/unattributed.ts +++ b/packages/core/src/check/unattributed.ts @@ -93,24 +93,23 @@ export function unattributedFindings( // 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 } : {}), - ...(cause.anchorOnly && !cause.config ? { root: sharedRoot(cause, filePrefix) } : {}), + ...(cause.anchorOnly && !cause.config ? { root: sharedRoot(cause) } : {}), severity: 'breaking', confidence: 1, fixability: cause.anchorOnly || cause.config ? 'assisted' : 'unknown', reason: cause.anchorOnly || cause.config ? cause.reason : UNATTRIBUTED_REASON, }; }); - return [...clustered, ...plainFindings(single, meta, surfaceB, unresolvedFiles, filePrefix)]; + return [...clustered, ...plainFindings(single, meta, surfaceB, unresolvedFiles)]; } -/** The declaration as the repository names it: a `../editor/x.ts` seen from a workspace is `packages/editor/x.ts`. */ -function sharedRoot(cause: DiagnosticCause, filePrefix: string): SharedRoot { - return { - name: cause.name, - file: posix.normalize(`${filePrefix}${cause.file}`), - line: cause.line, - reason: cause.reason, - }; +/** + * The declaration as the workspace names it (`../editor/x.ts`), like `usage.file`: the merge + * across workspaces prefixes both the same way, and `packages/tldraw/../editor/x.ts` + * normalizes to the repository path. + */ +function sharedRoot(cause: DiagnosticCause): SharedRoot { + return { name: cause.name, file: cause.file, line: cause.line, reason: cause.reason }; } function plainFindings( @@ -118,7 +117,6 @@ function plainFindings( meta: { package: string; from: string; to: string }, surfaceB: ApiSurface, unresolvedFiles: string[], - filePrefix: string, ): Finding[] { const unresolved = new Set(unresolvedFiles); // Leaf names declared in files the compiler could not fully resolve. @@ -154,7 +152,7 @@ function plainFindings( return { change, usage, - ...(d.root ? { root: sharedRoot(d.root, filePrefix) } : {}), + ...(d.root ? { root: sharedRoot(d.root) } : {}), severity: inconclusive ? 'unverified' : 'breaking', confidence: 1, fixability: 'unknown',