From 2eff60814d89b8cff4451df8715b18082872a1c6 Mon Sep 17 00:00:00 2001 From: John Dvorak Date: Thu, 21 May 2026 18:59:09 -0700 Subject: [PATCH] fix: force CDP extraction when formula uses variable-bound domains MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - 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 --- packages/imhotep-playwright/src/extraction.ts | 32 ++++++++++++++++--- 1 file changed, 28 insertions(+), 4 deletions(-) diff --git a/packages/imhotep-playwright/src/extraction.ts b/packages/imhotep-playwright/src/extraction.ts index 17edeaf..f09ee82 100644 --- a/packages/imhotep-playwright/src/extraction.ts +++ b/packages/imhotep-playwright/src/extraction.ts @@ -186,6 +186,7 @@ export function computeRequiredFacts(formulas: FormulaNode[]): { topology: boolean styles: boolean fragments: boolean + domAncestry: boolean } { const facts = new Set() for (const formula of formulas) { @@ -200,11 +201,33 @@ export function computeRequiredFacts(formulas: FormulaNode[]): { 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 { geometry: true, // Always required for subject resolution topology: facts.has('topology.stackingContextOf') || facts.has('subject.clipChain'), styles: facts.has('styles') || facts.has('computedStyle') || needsCssLengthMetrics, fragments: facts.has('subject.fragmentCount') || facts.has('subject.firstFragmentId'), + domAncestry: needsDomAncestry, } } @@ -355,7 +378,7 @@ export function attachMeasuredChWidths( export async function extractWorldFastGeometry( playwrightPage: Page, 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; errors: ImhotepDiagnostic[] }> { interface FastExtractedElement { tagName: string @@ -712,7 +735,7 @@ export async function extractWorldFastGeometry( export async function extractWorldCdp( playwrightPage: Page, 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; errors: ImhotepDiagnostic[] }> { const errors: ImhotepDiagnostic[] = [] const selectorToNodeIds = new Map() @@ -840,7 +863,7 @@ export async function extractWorld( playwrightPage: Page, selectors: string[], 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, ): Promise<{ world: GeometryWorld; selectorToIds: Map; errors: ImhotepDiagnostic[] }> { async function resolveViewport(): Promise<{ width: number; height: number }> { @@ -876,7 +899,7 @@ export async function extractWorld( const resolvedCacheDir = cacheDir === null ? null : (cacheDir ?? getDefaultCacheDir()) const cacheNamespace = getPageCacheNamespace(playwrightPage) 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}`] if (resolvedCacheDir !== null) { @@ -894,6 +917,7 @@ export async function extractWorld( const useFastGeometry = allowFastGeometry && requiredFacts?.geometry === true && requiredFacts.topology === false + && requiredFacts.domAncestry === false const result = useFastGeometry ? await extractWorldFastGeometry(playwrightPage, filteredSelectors, requiredFacts)