Skip to content

Commit c8d1bf8

Browse files
Bill LeoutsakosBill Leoutsakos
authored andcommitted
perf: bound indirect evidence after visual qualification
1 parent e62bc4c commit c8d1bf8

4 files changed

Lines changed: 88 additions & 17 deletions

File tree

scripts/design-diff/README.md

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,7 @@ JSON inputs are compared semantically, retaining array order and attributing cha
206206
the spec and configured renderer. Malformed/missing configured inputs flag with a limitation.
207207

208208
The graph reuses up to 32,768 import/export snapshots keyed by source blob, resolving their
209-
paths again for each revision. The resolver retains at most 32 parsed modules per revision,
209+
paths again for each revision. Each consumer resolver retains at most 32 parsed modules,
210210
and requests Bun garbage collection between parser batches. These resource controls do
211211
not change evidence or decisions. The Node-based test runner uses its own garbage collector.
212212

@@ -370,6 +370,14 @@ watchers distinguish changes to the forwarded export from unrelated declarations
370370
Custom props are exempt as event-only only when a resolved destructured prop is used exclusively
371371
to select JSX event handlers; the same rule applies inside nested JSX expressions. Unknown
372372
components and props with rendered uses keep conservative findings.
373+
Unchanged consumers are visited in dependency-distance order before source-path order, so a
374+
nearby visible use supplies the retained example before distant application plumbing when possible.
375+
The engine is a PR qualifier, not an exhaustive inventory of indirect effects. It analyzes every
376+
changed file, then follows unchanged consumers only while needed to establish whether the PR
377+
qualifies, retaining one nearby rendering-consumer examination when available. Once any retained
378+
finding flags and that examination is done, further indirect analysis cannot change the decision
379+
and is omitted with an explicit coverage limitation. Clean results still require all candidates
380+
to be examined. Direct findings, grouped source attribution and resolved usage counts remain.
373381

374382
Opaque runtime factories can still connect backend or authentication changes to UI inputs too
375383
broadly. These findings count as apparent false positives against the frozen nonvisual labels;

scripts/design-diff/analyze.ts

Lines changed: 31 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { cssValue, extractCss } from '#design-diff/extract/css'
55
import { extractDocument } from '#design-diff/extract/documents'
66
import { extractTsx } from '#design-diff/extract/tsx'
77
import { GitReader } from '#design-diff/git'
8-
import { causalSources, groupFindings } from '#design-diff/group'
8+
import { groupFindings } from '#design-diff/group'
99
import { renderingLock } from '#design-diff/infrastructure'
1010
import { fileLoadedInputs } from '#design-diff/inputs'
1111
import { reclaimMemory } from '#design-diff/memory'
@@ -192,25 +192,48 @@ export async function analyze(
192192
}
193193
let extracted = 0
194194
let omittedConsumers = 0
195-
const covered = new Set<string>()
196-
const currentPath = (file: string) => [...renames].find(([, old]) => old === file)?.[0] ?? file
195+
/** Prefer direct consumers before distant opaque application plumbing when retaining one example. */
196+
const downstream = new Map<string, Set<string>>()
197+
for (const graph of [before.graph, after.graph])
198+
for (const [file, dependencies] of graph.dependencies)
199+
for (const dependency of dependencies) {
200+
const consumers = downstream.get(dependency) ?? new Set<string>()
201+
consumers.add(file)
202+
downstream.set(dependency, consumers)
203+
}
204+
const distances = new Map([...changed].map((file) => [file, 0]))
205+
const queue = [...changed]
206+
for (let index = 0; index < queue.length; index++) {
207+
const file = queue[index]
208+
for (const consumer of downstream.get(file) ?? [])
209+
if (!distances.has(consumer)) {
210+
distances.set(consumer, distances.get(file)! + 1)
211+
queue.push(consumer)
212+
}
213+
}
197214
const ordered = [...affected].sort(
198-
(a, b) => Number(changed.has(b)) - Number(changed.has(a)) || a.localeCompare(b, 'en')
215+
(a, b) =>
216+
(distances.get(a) ?? Number.POSITIVE_INFINITY) -
217+
(distances.get(b) ?? Number.POSITIVE_INFINITY) || a.localeCompare(b, 'en')
199218
)
219+
let indirectExamples = 0
200220
for (const file of ordered) {
201221
if (++extracted % 32 === 0) reclaimMemory()
202222
if (!scoped(file, config) && !infrastructure(file, config)) continue
203223
if ([...renames.values()].includes(file) && !after.entries.has(file)) continue
204-
const roots = [...(causes.get(file) ?? [])].map(currentPath)
205-
if (!changed.has(file) && roots.length && roots.every((root) => covered.has(root))) {
224+
if (
225+
!changed.has(file) &&
226+
indirectExamples > 0 &&
227+
findings.some((finding) => finding.decision === 'flag')
228+
) {
206229
omittedConsumers++
207230
continue
208231
}
209-
const firstFinding = findings.length
210232
const oldFile = renames.get(file) ?? file
211233
const a = await extract(before, previousTailwind, oldFile)
212234
const b = await extract(after, nextTailwind, file)
213235
findings.push(...compareDefinitions(a, b, affected))
236+
if (!changed.has(file) && (a.length || b.length)) indirectExamples++
214237
if (
215238
changed.has(file) &&
216239
config.infrastructure.some((pattern) => new RegExp(pattern).test(file))
@@ -247,15 +270,11 @@ export async function analyze(
247270
review(file, after.entries.get(file)?.oid ?? '', 'Unsupported rendering mechanism')
248271
)
249272
)
250-
for (const change of findings.slice(firstFinding))
251-
if (change.decision === 'flag')
252-
for (const source of causalSources(change, causes, renames))
253-
if (source !== currentPath(file) || /\.[jt]sx$/.test(source)) covered.add(source)
254273
}
255274
if (omittedConsumers)
256275
report.limitations = [
257276
...report.limitations,
258-
`Repeated downstream expansion omitted for ${omittedConsumers} unchanged files after all contributing changed sources already had flagged evidence. Categories describe retained evidence; usage counts remain partial resolved references.`,
277+
`Indirect analysis omitted for ${omittedConsumers} unchanged files after the PR qualified and a nearby rendering consumer was examined. All in-scope changed files were analyzed; additional indirect effects are not exhaustively catalogued. Categories describe retained evidence; usage counts remain partial resolved references.`,
259278
]
260279
for (const file of changed) {
261280
if (file !== 'bun.lock' && !file.endsWith('/package.json') && file !== 'package.json')

scripts/design-diff/tests/group.test.ts

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,52 @@ const fixture = {
1818
'import {Icon} from "@sim/emcn";export const A=()=> <Icon className={dynamicStyle}/>',
1919
}
2020

21+
it('retains a nearby visual consumer before a distant opaque consumer', async () => {
22+
const token = 'apps/sim/token.ts'
23+
const direct = 'apps/sim/z-title.tsx'
24+
const report = await compareFiles(
25+
{
26+
[token]: 'export const title="Filling"',
27+
[direct]: 'import {title} from "./token";export const Title=()=> <span>{title}</span>',
28+
'apps/sim/helper.ts': 'import {title} from "./token";export const config=unknown(title)',
29+
'apps/sim/a-distant.tsx':
30+
'import {config} from "./helper";export const View=()=> <Widget config={config}/>',
31+
},
32+
{ [token]: 'export const title="Filling form"' },
33+
settings
34+
)
35+
expect(report.findings[0].example?.change.after?.location.file).toBe(direct)
36+
expect(report.findings[0].example?.change.category).toBe('content')
37+
})
38+
39+
it('keeps direct findings and declares partial indirect coverage after qualification', async () => {
40+
const token = 'apps/sim/token.ts'
41+
const other = 'apps/sim/other.tsx'
42+
const files = {
43+
[token]: 'export const colour="red"',
44+
[consumer]: 'export const View=()=> <div className="p-2"/>',
45+
[other]: 'import {colour} from "./token";export const Other=()=> <div style={{color:colour}}/>',
46+
'apps/sim/z-extra.tsx':
47+
'import {colour} from "./token";export const Extra=()=> <span style={{color:colour}}/>',
48+
}
49+
const report = await compareFiles(
50+
files,
51+
{
52+
[token]: 'export const colour="blue"',
53+
[consumer]: files[consumer].replace('p-2', 'p-4'),
54+
},
55+
settings
56+
)
57+
expect(report.flagged).toBe(true)
58+
expect(report.findings.some((finding) => finding.source.after?.file === consumer)).toBe(true)
59+
expect(report.limitations.join(' ')).toContain(
60+
'additional indirect effects are not exhaustively catalogued'
61+
)
62+
expect(
63+
(await compareFiles(files, { [token]: 'export const colour="blue"' }, settings)).flagged
64+
).toBe(true)
65+
})
66+
2167
it('groups a shared change once and counts only references to its defining module', async () => {
2268
const report = await compareFiles(
2369
fixture,

scripts/design-diff/tests/precision.test.ts

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -557,7 +557,7 @@ it('preserves selected helper return guards, defaults and call arguments', async
557557
expect((await compareFiles(files, { [view]: page('other') }, settings)).flagged).toBe(true)
558558
})
559559

560-
it('retains every changed source while avoiding repeated consumer expansion', async () => {
560+
it('retains direct changes and a nearby token example while bounding indirect expansion', async () => {
561561
const files = {
562562
[data]: 'export const colour="red"',
563563
[view]: 'import {colour} from "./data";export const Page=()=> <div style={{color:colour}}/>',
@@ -582,9 +582,7 @@ it('retains every changed source while avoiding repeated consumer expansion', as
582582
.referenceCount
583583
).toBe(2)
584584
expect(
585-
report.limitations.some((limitation) =>
586-
limitation.includes('Repeated downstream expansion omitted')
587-
)
585+
report.limitations.some((limitation) => limitation.includes('Indirect analysis omitted'))
588586
).toBe(true)
589587
})
590588

0 commit comments

Comments
 (0)