From a424d29cccbd5dd3860d74a6fd8235814c878a9b Mon Sep 17 00:00:00 2001 From: John Dvorak Date: Fri, 22 May 2026 11:55:58 -0700 Subject: [PATCH] =?UTF-8?q?fix:=20remove=20design-debt=20shims=20=E2=80=94?= =?UTF-8?q?=20falsy=20ID=20bug,=20selector=20normalization,=20concurrency,?= =?UTF-8?q?=20exception=20swallowing?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit pipeline.ts: || undefined → ?? undefined (9 occurrences) - || converts valid subject ID 0 to undefined because 0 is falsy in JS. This broke clause witnesses and topology references for the first subject. domain-index.ts: remove .toLowerCase() on CSS selectors - CSS selectors are case-sensitive (IDs, class names, attribute values). Lowercasing on lookup but not on storage (selectorIndex) meant case- sensitive selectors never matched — returning empty arrays silently. canonical.ts: add warning when visualBoxes falls back to layout boxes - visualBoxes ?? boxes silently substituted layout coordinates for visual space, producing incorrect results for transform-dependent assertions. Now emits console.warn so silent data corruption is visible. extraction.ts: serialize materializeSemanticSelector calls (3 sites) - Changed Promise.all over page.evaluate() to sequential for..of. While Playwright serializes CDP calls internally, concurrent DOM-modifying evaluate() calls create undefined execution order. Sequential resolution eliminates theoretical race conditions for semantic selector injection. engine.ts: include stack trace in evaluator exception diagnostics - Catch-all converted ALL exceptions (including TypeError from programming bugs) to IMH_EVALUATOR_EXCEPTION with just err.message. Now includes stack trace and logs to console.warn for visibility. Distinguishes TypeError (programming bug) from other evaluation errors. 648 SDK tests + 57 E2E hard tests pass, zero regressions. --- packages/imhotep-core/src/canonical.ts | 4 ++- packages/imhotep-core/src/pipeline.ts | 18 +++++----- packages/imhotep-geometry/src/domain-index.ts | 2 +- packages/imhotep-playwright/src/extraction.ts | 33 +++++++++---------- packages/imhotep-solver/src/engine.ts | 10 ++++-- 5 files changed, 36 insertions(+), 31 deletions(-) diff --git a/packages/imhotep-core/src/canonical.ts b/packages/imhotep-core/src/canonical.ts index c60a5ea..c12fb9e 100644 --- a/packages/imhotep-core/src/canonical.ts +++ b/packages/imhotep-core/src/canonical.ts @@ -778,7 +778,9 @@ export function adaptSolverWorldToCanonical( matrices: solverWorld.matrices ?? { values: [] }, rects: solverWorld.rects, boxes: solverWorld.boxes, - visualBoxes: solverWorld.visualBoxes ?? solverWorld.boxes, + visualBoxes: solverWorld.visualBoxes && solverWorld.visualBoxes.boxId.length > 0 + ? solverWorld.visualBoxes + : (console.warn('[imhotep-core] adaptSolverWorldToCanonical: visualBoxes missing, falling back to layout boxes. Visual-space assertions may produce incorrect results.'), solverWorld.boxes), fragments: { fragmentId: [], subjectId: [], diff --git a/packages/imhotep-core/src/pipeline.ts b/packages/imhotep-core/src/pipeline.ts index 2429d6f..52ce22c 100644 --- a/packages/imhotep-core/src/pipeline.ts +++ b/packages/imhotep-core/src/pipeline.ts @@ -802,8 +802,8 @@ export function executionIrToClauseDescriptors(context: ExecutionContext): Claus clauseKind: 'unknown', version: 1, subjectRef: executionIr.clauseSubject[i], - referenceRef: executionIr.clauseReference[i] || undefined, - frameRef: executionIr.clauseFrame[i] || undefined, + referenceRef: executionIr.clauseReference[i] ?? undefined, + frameRef: executionIr.clauseFrame[i] ?? undefined, options: { unsupported: true, rawType: clauseType }, }) continue @@ -837,13 +837,13 @@ export function executionIrToClauseDescriptors(context: ExecutionContext): Claus clauseId: `clause_${i}`, clauseKind, version: 1, - subjectRef: subjectRef || undefined, - referenceRef: referenceRef || undefined, - frameRef: executionIr.clauseFrame[i] || undefined, - stateRef: context.stateIds[executionIr.clauseState[i]] || undefined, - timelineRef: context.timelineIds[executionIr.clauseTimeline[i]] || undefined, - envGuardRef: context.envGuardIds[executionIr.clauseEnvGuard[i]] || undefined, - toleranceRef: context.toleranceIds[executionIr.clauseTolerance[i]] || undefined, + subjectRef: subjectRef ?? undefined, + referenceRef: referenceRef ?? undefined, + frameRef: executionIr.clauseFrame[i] ?? undefined, + stateRef: context.stateIds[executionIr.clauseState[i]] ?? undefined, + timelineRef: context.timelineIds[executionIr.clauseTimeline[i]] ?? undefined, + envGuardRef: context.envGuardIds[executionIr.clauseEnvGuard[i]] ?? undefined, + toleranceRef: context.toleranceIds[executionIr.clauseTolerance[i]] ?? undefined, bounds: Object.keys(bounds).length > 0 ? bounds : undefined, options: Object.keys(options).length > 0 ? options : undefined, }) diff --git a/packages/imhotep-geometry/src/domain-index.ts b/packages/imhotep-geometry/src/domain-index.ts index ffde03f..da5a9c5 100644 --- a/packages/imhotep-geometry/src/domain-index.ts +++ b/packages/imhotep-geometry/src/domain-index.ts @@ -16,7 +16,7 @@ import { GeometryWorld } from './world.js' * If the selector is not indexed, returns an empty array. */ export function getElementsBySelector(world: GeometryWorld, selector: string): number[] { - const normalized = selector.trim().toLowerCase() + const normalized = selector.trim() return world.selectorIndex.get(normalized) ?? [] } diff --git a/packages/imhotep-playwright/src/extraction.ts b/packages/imhotep-playwright/src/extraction.ts index 67d6301..b8a5c23 100644 --- a/packages/imhotep-playwright/src/extraction.ts +++ b/packages/imhotep-playwright/src/extraction.ts @@ -409,12 +409,11 @@ export async function extractWorldFastGeometry( selectorToIds: Array<[string, number[]]> } - const selectorPlans: SelectorPlan[] = await Promise.all( - selectors.map(async (key, i) => { - const queries = await materializeSemanticSelector(playwrightPage, key, i) - return { key, queries } - }), - ) + const selectorPlans: SelectorPlan[] = [] + for (let i = 0; i < selectors.length; i++) { + const queries = await materializeSemanticSelector(playwrightPage, selectors[i], i) + selectorPlans.push({ key: selectors[i], queries }) + } try { const extracted = await playwrightPage.evaluate(({ plans, needs }: any) => { @@ -800,12 +799,11 @@ export async function extractWorldCdp( const errors: ImhotepDiagnostic[] = [] const selectorToNodeIds = new Map() - const selectorPlans: SelectorPlan[] = await Promise.all( - selectors.map(async (key, i) => { - const queries = await materializeSemanticSelector(playwrightPage, key, i) - return { key, queries } - }), - ) + const selectorPlans: SelectorPlan[] = [] + for (let i = 0; i < selectors.length; i++) { + const queries = await materializeSemanticSelector(playwrightPage, selectors[i], i) + selectorPlans.push({ key: selectors[i], queries }) + } const sessionManager = createSessionManager(playwrightPage) try { @@ -993,12 +991,11 @@ export async function extractWorld( if (requiredFacts?.styles) { try { - const plans: SelectorPlan[] = await Promise.all( - filteredSelectors.map(async (key, i) => { - const queries = await materializeSemanticSelector(playwrightPage, key, i) - return { key, queries } - }), - ) + const plans: SelectorPlan[] = [] + for (let i = 0; i < filteredSelectors.length; i++) { + const queries = await materializeSemanticSelector(playwrightPage, filteredSelectors[i], i) + plans.push({ key: filteredSelectors[i], queries }) + } const chWidthsBySelector = await measureChWidthsByPlan(playwrightPage, plans) attachMeasuredChWidths(result.world, result.selectorToIds, chWidthsBySelector) } catch { diff --git a/packages/imhotep-solver/src/engine.ts b/packages/imhotep-solver/src/engine.ts index 131ea92..ff6e69d 100644 --- a/packages/imhotep-solver/src/engine.ts +++ b/packages/imhotep-solver/src/engine.ts @@ -272,7 +272,13 @@ export function evaluate( diagnostics.push(...result.diagnostics); } } catch (err) { - const message = err instanceof Error ? err.message : String(err); + const message = err instanceof Error ? err.message : String(err) + const stack = err instanceof Error ? err.stack : undefined + console.warn(`[imhotep-solver] clause evaluator exception for kind "${clause.clauseKind}" (id ${clause.clauseId}): ${message}`) + if (stack) console.warn(stack) + const detail = err instanceof Error && err.name === 'TypeError' + ? `UNCAUGHT PROGRAMMING BUG: ${message}\n${stack ?? ''}` + : `Evaluator error: ${message}` const result: ClauseResult = { clauseId: clause.clauseId, status: 'error', @@ -282,7 +288,7 @@ export function evaluate( code: 'IMH_EVALUATOR_EXCEPTION', severity: 'error', category: 'internal-error', - message, + message: detail, clauseId: clause.clauseId, }, ],