Skip to content

Commit e62bc4c

Browse files
Bill LeoutsakosBill Leoutsakos
authored andcommitted
fix: retain captured visual inputs and reduce analysis overhead
1 parent 8697761 commit e62bc4c

7 files changed

Lines changed: 217 additions & 41 deletions

File tree

scripts/design-diff/README.md

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -363,6 +363,20 @@ ground truth. Corrections must be documented separately rather than rewriting fr
363363
Holdout PRs #6986 and #6929 were inspected while debugging resolver precision and resource
364364
use after labels were frozen, so this is not a wholly untouched blind evaluation.
365365

366+
Import-graph construction uses syntax parsing without repeatedly running expression/refactor
367+
normalization. Bounded expression fallbacks retain captured helper inputs (including progress
368+
title maps), select statically known properties, and exclude type-only references. Re-export
369+
watchers distinguish changes to the forwarded export from unrelated declarations in its module.
370+
Custom props are exempt as event-only only when a resolved destructured prop is used exclusively
371+
to select JSX event handlers; the same rule applies inside nested JSX expressions. Unknown
372+
components and props with rendered uses keep conservative findings.
373+
374+
Opaque runtime factories can still connect backend or authentication changes to UI inputs too
375+
broadly. These findings count as apparent false positives against the frozen nonvisual labels;
376+
an unresolved finding is not proof that pixels changed. Qualification and false-positive rates
377+
must be reported separately, with failed comparisons excluded from neither the failure count
378+
nor the denominator disclosure.
379+
366380
```sh
367381
bun --no-env-file scripts/design-diff/benchmark.ts \
368382
--engine /path/to/clean/engine-checkout --sha <immutable-engine-commit> \

scripts/design-diff/ast.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,14 +11,19 @@ export const traverse: typeof traverseModule =
1111
? traverseModule
1212
: (traverseModule as unknown as { default: typeof traverseModule }).default
1313

14-
export function parseSource(source: string, file: string) {
15-
const ast = parse(source, {
14+
/** Parse import/reference syntax without doing the resolver's literal/refactor work. */
15+
export function parseSyntax(source: string, file: string) {
16+
return parse(source, {
1617
sourceType: 'unambiguous',
1718
sourceFilename: file,
1819
plugins: ['jsx', 'typescript', 'decorators-legacy'],
1920
errorRecovery: false,
2021
attachComment: false,
2122
})
23+
}
24+
25+
export function parseSource(source: string, file: string) {
26+
const ast = parseSyntax(source, file)
2227
normalizeRefactors(ast, traverse)
2328
normalizeLiteralAliases(ast, traverse)
2429
return ast

scripts/design-diff/dependencies.ts

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import type { NodePath } from '@babel/traverse'
22
import * as t from '@babel/types'
3-
import { fingerprint, location, parseSource, propertyName, traverse } from '#design-diff/ast'
3+
import { fingerprint, location, parseSyntax, propertyName, traverse } from '#design-diff/ast'
44
import { environmentFields } from '#design-diff/environment'
55
import { finiteKeys } from '#design-diff/finite'
66
import { reclaimMemory } from '#design-diff/memory'
@@ -100,7 +100,7 @@ export class DependencyGraph {
100100
if (target) raw.add(target)
101101
}
102102
try {
103-
const ast = parseSource(source, file)
103+
const ast = parseSyntax(source, file)
104104
const exports = new Map<string, ExportTarget>()
105105
const imports = new Map<string, ExportTarget>()
106106
const stars: string[] = []
@@ -314,10 +314,7 @@ export class DependencyGraph {
314314
if (precise) {
315315
for (const origin of trace.origins) edge(origin.file, origin.symbol)
316316
for (const route of trace.routes) {
317-
if (!this.modules.get(route)?.barrel) {
318-
if (!trace.origins.some((origin) => origin.file === route)) edge(route, '*')
319-
continue
320-
}
317+
if (trace.origins.some((origin) => origin.file === route)) continue
321318
const watchers = this.watches.get(route) ?? new Map<string, Set<string>>()
322319
const signatures = watchers.get(file) ?? new Set<string>()
323320
signatures.add(
@@ -352,7 +349,7 @@ export class DependencyGraph {
352349
for (const target of this.raw.get(file) ?? []) this.dependencies.get(file)!.add(target)
353350
return
354351
}
355-
const ast = parseSource(this.tree.texts.get(file) as string, file)
352+
const ast = parseSyntax(this.tree.texts.get(file) as string, file)
356353
const namespaces = new Map([...module.imports].filter(([, target]) => target.name === '*'))
357354
const referenced = new Set<string>()
358355
traverse(ast, {
@@ -418,7 +415,7 @@ export class DependencyGraph {
418415
private count(file: string) {
419416
if (this.counted.has(file) || this.modules.get(file)?.barrel) return
420417
this.counted.add(file)
421-
const ast = parseSource(this.tree.texts.get(file) as string, file)
418+
const ast = parseSyntax(this.tree.texts.get(file) as string, file)
422419
const record = (specifier: string, name: string, references: NodePath[]) => {
423420
const target = this.tree.resolve(file, specifier)
424421
if (!target) return
@@ -488,8 +485,8 @@ export class DependencyGraph {
488485
const b = other.watches.get(root)
489486
for (const file of new Set([...(a?.keys() ?? []), ...(b?.keys() ?? [])])) {
490487
if (
491-
this.modules.get(root)?.barrel &&
492-
other.modules.get(root)?.barrel &&
488+
this.modules.has(root) &&
489+
other.modules.has(root) &&
493490
JSON.stringify([...(a?.get(file) ?? [])].sort()) ===
494491
JSON.stringify([...(b?.get(file) ?? [])].sort())
495492
)
@@ -598,7 +595,7 @@ export class DependencyGraph {
598595
const unowned = new Set<string>()
599596
const members = new Map<string, { keys?: string[]; owners?: string[] }[]>()
600597
const resolver = new Resolver(this.tree)
601-
const ast = parseSource(source, file)
598+
const ast = parseSyntax(source, file)
602599
traverse(ast, {
603600
Program(p) {
604601
const entries = Object.entries(p.scope.bindings)

scripts/design-diff/extract/tsx.ts

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -143,7 +143,14 @@ export function extractTsx(resolver: Resolver, file: string): Definition[] {
143143
})
144144
const attributeOrder = opening.attributes
145145
.filter(
146-
(attr) => !t.isJSXAttribute(attr) || !nonvisualAttributes.test(propertyName(attr.name))
146+
(attr) =>
147+
!t.isJSXAttribute(attr) ||
148+
(!nonvisualAttributes.test(propertyName(attr.name)) &&
149+
!resolver.eventOnlyProp(
150+
child(child(path, 'openingElement'), 'name'),
151+
propertyName(attr.name),
152+
file
153+
))
147154
)
148155
.map((attr) => (t.isJSXAttribute(attr) ? propertyName(attr.name) : '...spread'))
149156
const evidence = literal({ tag: name, children: childShapes, attributeOrder })
@@ -190,6 +197,7 @@ export function extractTsx(resolver: Resolver, file: string): Definition[] {
190197
JSXAttribute(path) {
191198
const name = propertyName(path.node.name)
192199
if (nonvisualAttributes.test(name)) return
200+
if (resolver.eventOnlyProp(child(path.parentPath, 'name'), name, file)) return
193201
const value = child(path, 'value')
194202
const evidence = value.node ? resolver.evaluate(value, file) : literal(true)
195203
if (

scripts/design-diff/resolve.ts

Lines changed: 111 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -40,12 +40,14 @@ export class Resolver {
4040
private readonly evaluations = new WeakMap<t.Node, Evidence>()
4141
private unknowns = new WeakMap<t.Node, Map<string, Evidence>>()
4242
private readonly opaqueValues = new Map<string, Evidence>()
43+
private readonly resolvingExports = new Set<string>()
4344
private readonly resolvingUnknown = new Set<t.Node>()
4445
private steps = 0
4546
private parsedModules = 0
4647
private readonly active = new Set<t.Node>()
4748
private dependencies = new Set<string>()
4849
private unresolved = new Set<string>()
50+
private readonly eventProps = new WeakMap<t.Node, Set<string>>()
4951

5052
constructor(
5153
readonly tree: SourceTree,
@@ -295,7 +297,7 @@ export class Resolver {
295297
'Configured capability adapter: declared environment fields and provider factories are traced without execution'
296298
)
297299
return {
298-
$environmentAdapter: this.opaqueExports(adapter.module, adapter.export),
300+
$environmentAdapter: fingerprint(this.module(adapter.module).ast.program),
299301
implementation,
300302
environment: selected,
301303
arguments: children(path, 'arguments').map((argument) =>
@@ -331,6 +333,44 @@ export class Resolver {
331333
return { value: this.value(input, file, depth + 1), conditions }
332334
}
333335

336+
/** Resolve props whose only uses select event handlers, without assuming custom prop names. */
337+
eventOnlyProp(name: NodePath, property: string, file: string): boolean {
338+
if (!name.isJSXIdentifier() || !/^[A-Z]/.test(name.node.name)) return false
339+
const key = name.node
340+
let properties = this.eventProps.get(key)
341+
if (!properties) {
342+
properties = new Set<string>()
343+
this.eventProps.set(key, properties)
344+
const target = this.callable(name, file)
345+
if (!target) return false
346+
const parameter = children(target.path, 'params')[0]
347+
if (!parameter?.isObjectPattern()) return false
348+
for (const prop of children(parameter, 'properties')) {
349+
if (!prop.isObjectProperty() || prop.node.computed) continue
350+
const value = child(prop, 'value')
351+
const local = value.isAssignmentPattern() ? child(value, 'left') : value
352+
if (!local.isIdentifier()) continue
353+
const references = local.scope
354+
.getBinding(local.node.name)
355+
?.referencePaths.filter(
356+
(reference) => !reference.findParent((parent) => parent.isTSType())
357+
)
358+
if (!references?.length) continue
359+
if (
360+
references.every((reference) => {
361+
for (let parent = reference.parentPath; parent; parent = parent.parentPath) {
362+
if (parent.isFunction() || parent.isCallExpression()) return false
363+
if (parent.isJSXAttribute()) return /^on[A-Z]/.test(propertyName(parent.node.name))
364+
}
365+
return false
366+
})
367+
)
368+
properties.add(propertyName(prop.node.key))
369+
}
370+
}
371+
return properties.has(property)
372+
}
373+
334374
private currentFile = ''
335375

336376
/** Select a property before expanding siblings, including createEnv's schema convention. */
@@ -492,7 +532,7 @@ export class Resolver {
492532
if (path.isFunction()) return { path, file }
493533
if (path.isTSAsExpression() || path.isTSSatisfiesExpression() || path.isTSNonNullExpression())
494534
return this.callable(child(path, 'expression'), file, seen)
495-
if (!path.isIdentifier()) return undefined
535+
if (!path.isIdentifier() && !path.isJSXIdentifier()) return undefined
496536
const binding = path.scope.getBinding(path.node.name)
497537
if (!binding?.constant) return undefined
498538
if (binding.path.isFunctionDeclaration()) return { path: binding.path, file }
@@ -691,24 +731,42 @@ export class Resolver {
691731
})
692732
return renders
693733
? { $renderFunction: { file, symbol: symbolName(path) } }
694-
: fingerprint(path.node)
734+
: this.unknown(path, 'Opaque helper input is traced without execution', file)
695735
}
696736

697737
/** Opaque export summaries are independent of expression budgets and parser cache eviction. */
698738
private opaqueExports(file: string, name: string): Data {
699739
const key = `${file}:${name}`
740+
if (this.affected && !this.affected.has(file)) {
741+
this.dependencies.add(file)
742+
return { $unchangedInput: { file, export: name } }
743+
}
700744
const cached = this.opaqueValues.get(key)
701745
if (cached) {
702746
for (const file of cached.dependencies) this.dependencies.add(file)
703747
return cached.value
704748
}
749+
this.dependencies.add(file)
750+
if (this.resolvingExports.has(key)) {
751+
this.unresolved.add('Opaque export dependency cycle')
752+
return { $cycle: key }
753+
}
754+
if (this.resolvingExports.size >= 64) {
755+
this.unresolved.add('Opaque export dependency depth limit')
756+
return { $unresolvedExport: key, blob: this.tree.entries.get(file)?.oid ?? null }
757+
}
758+
this.resolvingExports.add(key)
705759
const dependencies = this.dependencies
706760
this.dependencies = new Set([file])
707-
const value = this.opaqueExportInner(file, name)
708-
this.opaqueValues.set(key, { value, dependencies: [...this.dependencies], unresolved: [] })
709-
for (const file of this.dependencies) dependencies.add(file)
710-
this.dependencies = dependencies
711-
return value
761+
try {
762+
const value = this.opaqueExportInner(file, name)
763+
this.opaqueValues.set(key, { value, dependencies: [...this.dependencies], unresolved: [] })
764+
return value
765+
} finally {
766+
for (const file of this.dependencies) dependencies.add(file)
767+
this.dependencies = dependencies
768+
this.resolvingExports.delete(key)
769+
}
712770
}
713771

714772
/** Follow export declarations only when normal evaluation is bounded or ambiguous. */
@@ -770,11 +828,50 @@ export class Resolver {
770828
const dependencies = this.dependencies
771829
this.dependencies = new Set([file])
772830
const inputs = new Map<string, Data>()
831+
const inspected = new Set<t.Node>()
773832
const inspect = (reference: NodePath) => {
774833
if (!reference.isReferencedIdentifier()) return
834+
if (reference.findParent((parent) => parent.isTSType())) return
775835
const binding = reference.scope.getBinding(reference.node.name)
776836
if (!binding) return
777837
const bound = binding.path
838+
if (bound === path || bound.findParent((parent) => parent === path)) return
839+
const member = reference.parentPath
840+
if (member?.isMemberExpression() && member.node.object === reference.node) {
841+
const selection = member.node.computed
842+
? finiteKeys(child(member, 'property'), file, this)
843+
: { keys: [propertyName(member.node.property)], dependencies: [] }
844+
if (selection?.keys) {
845+
for (const dependency of selection.dependencies) this.dependencies.add(dependency)
846+
const values = selection.keys.map((key) => this.selected(reference, String(key), file, 0))
847+
if (values.every((value) => value !== undefined)) {
848+
inputs.set(`selected:${reference.node.name}.[${selection.keys.join(',')}]`, {
849+
$finiteSelection: selection.keys.map((key, index) => [key, values[index]!] as Data),
850+
})
851+
return
852+
}
853+
}
854+
}
855+
if (
856+
!bound.isImportSpecifier() &&
857+
!bound.isImportDefaultSpecifier() &&
858+
!bound.isImportNamespaceSpecifier()
859+
) {
860+
if (inspected.has(bound.node)) return
861+
inspected.add(bound.node)
862+
const parameter = this.parameter(bound, reference.node.name, file, 0)
863+
if (parameter !== undefined) {
864+
inputs.set(`parameter:${reference.node.name}`, parameter)
865+
return
866+
}
867+
const value = bound.isVariableDeclarator() ? child(bound, 'init') : bound
868+
if (value.node)
869+
inputs.set(
870+
`local:${reference.node.name}`,
871+
this.unknown(value, 'Captured input feeds unresolved rendering', file)
872+
)
873+
return
874+
}
778875
if (
779876
bound.isImportSpecifier() ||
780877
bound.isImportDefaultSpecifier() ||
@@ -790,25 +887,11 @@ export class Resolver {
790887
? 'default'
791888
: '*'
792889
this.dependencies.add(target)
793-
const member = reference.parentPath
794-
if (member?.isMemberExpression() && member.node.object === reference.node) {
795-
const selection = member.node.computed
796-
? finiteKeys(child(member, 'property'), file, this)
797-
: { keys: [propertyName(member.node.property)], dependencies: [] }
798-
if (selection?.keys) {
799-
for (const file of selection.dependencies) this.dependencies.add(file)
800-
const values = selection.keys.map((key) =>
801-
this.selected(reference, String(key), file, 0)
802-
)
803-
if (values.every((value) => value !== undefined)) {
804-
inputs.set(`${target}:${name}.[${selection.keys.join(',')}]`, {
805-
$finiteSelection: selection.keys.map((key, index) => [key, values[index]!] as Data),
806-
})
807-
return
808-
}
809-
}
810-
}
811890
const origin = this.tree.graph?.resolvedExport(target, name)?.origin
891+
if (this.affected && origin && !this.affected.has(origin.file)) {
892+
inputs.set(`${target}:${name}`, this.opaqueExports(origin.file, origin.exported))
893+
return
894+
}
812895
inputs.set(
813896
`${target}:${name}`,
814897
origin
@@ -858,7 +941,7 @@ export class Resolver {
858941
const key = `${file}:${name}`
859942
if (depth > this.tree.config.limits.resolutionDepth || visited.has(key)) {
860943
this.unresolved.add(visited.has(key) ? 'Dependency cycle' : 'Export resolution depth limit')
861-
return { $unresolved: key }
944+
return this.opaqueExports(file, name)
862945
}
863946
visited.add(key)
864947
this.dependencies.add(file)
@@ -960,6 +1043,7 @@ export class Resolver {
9601043
if (attribute.isJSXAttribute()) {
9611044
const name = propertyName(attribute.node.name)
9621045
if (/^(?:key|ref|on[A-Z].*)$/.test(name)) continue
1046+
if (this.eventOnlyProp(child(opening, 'name'), name, file)) continue
9631047
attributes.push([
9641048
name,
9651049
attribute.node.value ? this.value(child(attribute, 'value'), file, depth + 1) : true,

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

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,26 @@ const index = 'apps/sim/index.ts'
1010
const consumer = 'apps/sim/page.tsx'
1111
const unrelated = 'apps/sim/unrelated.tsx'
1212

13+
it('keeps re-exports independent of other declarations in a non-barrel module', async () => {
14+
const files = {
15+
[token]: 'export const colour="red";export {size} from "./other"',
16+
[other]: 'export const size=4',
17+
[consumer]: 'import {size} from "./token";export const Page=()=> <div style={{width:size}}/>',
18+
}
19+
expect(
20+
(
21+
await compareFiles(
22+
files,
23+
{ [token]: 'export const colour="blue";export {size} from "./other"' },
24+
settings
25+
)
26+
).flagged
27+
).toBe(false)
28+
expect((await compareFiles(files, { [other]: 'export const size=8' }, settings)).flagged).toBe(
29+
true
30+
)
31+
})
32+
1333
it.each([
1434
[
1535
'named alias',

0 commit comments

Comments
 (0)