fix: force CDP extraction when formula uses variable-bound domains
- computeRequiredFacts now returns domAncestry flag by scanning formula
bindings for parentVar references (descendants/children domains)
- extractWorld fast-path gate now requires domAncestry === false — formulas
with parentVar domains will always use CDP extraction, which provides
the DOM parentNodeId data needed for ancestor index construction
- Prevents silent indeterminate results when descendants(, sel) is
used on the fast path (which lacks DOM ancestry data)
- Cache key updated to include domAncestry flag ('a') so cached fast vs
CDP results for the same selectors don't collide
This commit is contained in:
@@ -186,6 +186,7 @@ export function computeRequiredFacts(formulas: FormulaNode[]): {
|
|||||||
topology: boolean
|
topology: boolean
|
||||||
styles: boolean
|
styles: boolean
|
||||||
fragments: boolean
|
fragments: boolean
|
||||||
|
domAncestry: boolean
|
||||||
} {
|
} {
|
||||||
const facts = new Set<string>()
|
const facts = new Set<string>()
|
||||||
for (const formula of formulas) {
|
for (const formula of formulas) {
|
||||||
@@ -200,11 +201,33 @@ export function computeRequiredFacts(formulas: FormulaNode[]): {
|
|||||||
|
|
||||||
const needsCssLengthMetrics = formulas.some((formula) => formulaNeedsCssLengthMetrics(formula))
|
const needsCssLengthMetrics = formulas.some((formula) => formulaNeedsCssLengthMetrics(formula))
|
||||||
|
|
||||||
|
const needsDomAncestry = formulas.some((formula) => {
|
||||||
|
let found = false
|
||||||
|
const scan = (node: FormulaNode) => {
|
||||||
|
if (found) return
|
||||||
|
if (node.kind === 'forall' || node.kind === 'exists') {
|
||||||
|
for (const b of node.bindings) {
|
||||||
|
if (b.domain.parentVar) { found = true; return }
|
||||||
|
}
|
||||||
|
scan(node.body)
|
||||||
|
} else if (node.kind === 'and' || node.kind === 'or') {
|
||||||
|
scan(node.left); scan(node.right)
|
||||||
|
} else if (node.kind === 'not') {
|
||||||
|
scan(node.operand)
|
||||||
|
} else if (node.kind === 'implies') {
|
||||||
|
scan(node.antecedent); scan(node.consequent)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
scan(formula)
|
||||||
|
return found
|
||||||
|
})
|
||||||
|
|
||||||
return {
|
return {
|
||||||
geometry: true, // Always required for subject resolution
|
geometry: true, // Always required for subject resolution
|
||||||
topology: facts.has('topology.stackingContextOf') || facts.has('subject.clipChain'),
|
topology: facts.has('topology.stackingContextOf') || facts.has('subject.clipChain'),
|
||||||
styles: facts.has('styles') || facts.has('computedStyle') || needsCssLengthMetrics,
|
styles: facts.has('styles') || facts.has('computedStyle') || needsCssLengthMetrics,
|
||||||
fragments: facts.has('subject.fragmentCount') || facts.has('subject.firstFragmentId'),
|
fragments: facts.has('subject.fragmentCount') || facts.has('subject.firstFragmentId'),
|
||||||
|
domAncestry: needsDomAncestry,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -355,7 +378,7 @@ export function attachMeasuredChWidths(
|
|||||||
export async function extractWorldFastGeometry(
|
export async function extractWorldFastGeometry(
|
||||||
playwrightPage: Page,
|
playwrightPage: Page,
|
||||||
selectors: string[],
|
selectors: string[],
|
||||||
requiredFacts?: { geometry: boolean; topology: boolean; styles: boolean; fragments: boolean },
|
requiredFacts?: { geometry: boolean; topology: boolean; styles: boolean; fragments: boolean; domAncestry: boolean },
|
||||||
): Promise<{ world: GeometryWorld; selectorToIds: Map<string, number[]>; errors: ImhotepDiagnostic[] }> {
|
): Promise<{ world: GeometryWorld; selectorToIds: Map<string, number[]>; errors: ImhotepDiagnostic[] }> {
|
||||||
interface FastExtractedElement {
|
interface FastExtractedElement {
|
||||||
tagName: string
|
tagName: string
|
||||||
@@ -712,7 +735,7 @@ export async function extractWorldFastGeometry(
|
|||||||
export async function extractWorldCdp(
|
export async function extractWorldCdp(
|
||||||
playwrightPage: Page,
|
playwrightPage: Page,
|
||||||
selectors: string[],
|
selectors: string[],
|
||||||
requiredFacts?: { geometry: boolean; topology: boolean; styles: boolean; fragments: boolean },
|
requiredFacts?: { geometry: boolean; topology: boolean; styles: boolean; fragments: boolean; domAncestry: boolean },
|
||||||
): Promise<{ world: GeometryWorld; selectorToIds: Map<string, number[]>; errors: ImhotepDiagnostic[] }> {
|
): Promise<{ world: GeometryWorld; selectorToIds: Map<string, number[]>; errors: ImhotepDiagnostic[] }> {
|
||||||
const errors: ImhotepDiagnostic[] = []
|
const errors: ImhotepDiagnostic[] = []
|
||||||
const selectorToNodeIds = new Map<string, number[]>()
|
const selectorToNodeIds = new Map<string, number[]>()
|
||||||
@@ -840,7 +863,7 @@ export async function extractWorld(
|
|||||||
playwrightPage: Page,
|
playwrightPage: Page,
|
||||||
selectors: string[],
|
selectors: string[],
|
||||||
cacheDir?: string | null,
|
cacheDir?: string | null,
|
||||||
requiredFacts?: { geometry: boolean; topology: boolean; styles: boolean; fragments: boolean },
|
requiredFacts?: { geometry: boolean; topology: boolean; styles: boolean; fragments: boolean; domAncestry: boolean },
|
||||||
allowFastGeometry = false,
|
allowFastGeometry = false,
|
||||||
): Promise<{ world: GeometryWorld; selectorToIds: Map<string, number[]>; errors: ImhotepDiagnostic[] }> {
|
): Promise<{ world: GeometryWorld; selectorToIds: Map<string, number[]>; errors: ImhotepDiagnostic[] }> {
|
||||||
async function resolveViewport(): Promise<{ width: number; height: number }> {
|
async function resolveViewport(): Promise<{ width: number; height: number }> {
|
||||||
@@ -876,7 +899,7 @@ export async function extractWorld(
|
|||||||
const resolvedCacheDir = cacheDir === null ? null : (cacheDir ?? getDefaultCacheDir())
|
const resolvedCacheDir = cacheDir === null ? null : (cacheDir ?? getDefaultCacheDir())
|
||||||
const cacheNamespace = getPageCacheNamespace(playwrightPage)
|
const cacheNamespace = getPageCacheNamespace(playwrightPage)
|
||||||
const cacheSelectors = requiredFacts
|
const cacheSelectors = requiredFacts
|
||||||
? [...filteredSelectors, `__imhotep_facts:${requiredFacts.geometry ? 'g' : ''}${requiredFacts.topology ? 't' : ''}${requiredFacts.styles ? 's' : ''}${requiredFacts.fragments ? 'f' : ''}:${allowFastGeometry ? 'fast' : 'cdp'}`, `__imhotep_page_ns:${cacheNamespace}`]
|
? [...filteredSelectors, `__imhotep_facts:${requiredFacts.geometry ? 'g' : ''}${requiredFacts.topology ? 't' : ''}${requiredFacts.styles ? 's' : ''}${requiredFacts.fragments ? 'f' : ''}${requiredFacts.domAncestry ? 'a' : ''}:${allowFastGeometry ? 'fast' : 'cdp'}`, `__imhotep_page_ns:${cacheNamespace}`]
|
||||||
: [...filteredSelectors, `__imhotep_page_ns:${cacheNamespace}`]
|
: [...filteredSelectors, `__imhotep_page_ns:${cacheNamespace}`]
|
||||||
|
|
||||||
if (resolvedCacheDir !== null) {
|
if (resolvedCacheDir !== null) {
|
||||||
@@ -894,6 +917,7 @@ export async function extractWorld(
|
|||||||
const useFastGeometry = allowFastGeometry
|
const useFastGeometry = allowFastGeometry
|
||||||
&& requiredFacts?.geometry === true
|
&& requiredFacts?.geometry === true
|
||||||
&& requiredFacts.topology === false
|
&& requiredFacts.topology === false
|
||||||
|
&& requiredFacts.domAncestry === false
|
||||||
|
|
||||||
const result = useFastGeometry
|
const result = useFastGeometry
|
||||||
? await extractWorldFastGeometry(playwrightPage, filteredSelectors, requiredFacts)
|
? await extractWorldFastGeometry(playwrightPage, filteredSelectors, requiredFacts)
|
||||||
|
|||||||
Reference in New Issue
Block a user