From 066ef9f6777a3ff759b141c1823230626bd24577 Mon Sep 17 00:00:00 2001 From: John Dvorak Date: Fri, 22 May 2026 15:44:44 -0700 Subject: [PATCH] =?UTF-8?q?refactor:=20remove=20global=20registry=20fallba?= =?UTF-8?q?cks=20=E2=80=94=20factory=20pattern=20for=20test=20isolation?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit predicates.ts: Add populateDefaultPredicates(registry) accepting any PredicateRegistry. Add createDefaultPredicateRegistry() factory. registerDefaultPredicates() now delegates to populateDefaultPredicates on the global (backward compatible). logic-engine.ts: Replace globalPredicateRegistry fallback with createDefaultPredicateRegistry() factory. Each evaluateLogic() call creates a fresh self-populated registry unless one is explicitly injected. No shared mutable state between evaluations. engine.ts: Same pattern for clauses — add populateDefaultClauses(registry), createDefaultClauseRegistry() factory. registerDefaultClauses() now delegates to populateDefaultClauses on the global. evaluate() replaces globalClauseRegistry fallback with createDefaultClauseRegistry(). registry.ts: @deprecated tag on registerClause with migration note. Both global registries remain for backward compatibility via the deprecated registerDefault*() functions, but the evaluation engines no longer depend on them. Every evaluation gets its own registry by default, so custom predicates registered by one test cannot leak into another. Tests using explicit registry injection are unaffected. 662 tests pass (315 DSL + 141 core + 149 solver + 57 E2E). --- packages/imhotep-solver/src/engine.ts | 87 +++++++++++++-------- packages/imhotep-solver/src/index.ts | 4 + packages/imhotep-solver/src/logic-engine.ts | 4 +- packages/imhotep-solver/src/predicates.ts | 87 ++++++++++++--------- packages/imhotep-solver/src/registry.ts | 2 +- 5 files changed, 109 insertions(+), 75 deletions(-) diff --git a/packages/imhotep-solver/src/engine.ts b/packages/imhotep-solver/src/engine.ts index ff6e69d..87711f9 100644 --- a/packages/imhotep-solver/src/engine.ts +++ b/packages/imhotep-solver/src/engine.ts @@ -72,100 +72,119 @@ export interface EvaluationOutput { } /** - * Register all built-in clause evaluators with the global registry. - * Call this once before invoking evaluate() if you want the default set. + * Populate a ClauseRegistry with all built-in clause evaluators (idempotent). */ -export function registerDefaultClauses(): void { - registerClause({ +export function populateDefaultClauses(registry: ClauseRegistry): void { + const SENTINEL = '__imhotep_clause_defaults__' + if (registry.getEvaluator(SENTINEL)) return + registry.register({ + clauseKind: SENTINEL, + version: 1, + requiredFacts: [], + evaluate: () => ({ clauseId: '', status: 'pass' as const, truth: 'determinate' as const }), + }) + registry.register({ clauseKind: 'relation.leftOf', version: 1, requiredFacts: ['subject.primaryBox', 'reference.primaryBox'], evaluate: evaluateLeftOf, - }); - registerClause({ + }) + registry.register({ clauseKind: 'relation.rightOf', version: 1, requiredFacts: ['subject.primaryBox', 'reference.primaryBox'], evaluate: evaluateRightOf, - }); - registerClause({ + }) + registry.register({ clauseKind: 'relation.above', version: 1, requiredFacts: ['subject.primaryBox', 'reference.primaryBox'], evaluate: evaluateAbove, - }); - registerClause({ + }) + registry.register({ clauseKind: 'relation.below', version: 1, requiredFacts: ['subject.primaryBox', 'reference.primaryBox'], evaluate: evaluateBelow, - }); - registerClause({ + }) + registry.register({ clauseKind: 'relation.overlaps', version: 1, requiredFacts: ['subject.primaryBox', 'reference.primaryBox'], evaluate: evaluateOverlaps, - }); - registerClause({ + }) + registry.register({ clauseKind: 'relation.inside', version: 1, requiredFacts: ['subject.primaryBox', 'reference.primaryBox'], evaluate: evaluateInside, - }); - registerClause({ + }) + registry.register({ clauseKind: 'alignment.alignedWith', version: 1, requiredFacts: ['subject.primaryBox', 'reference.primaryBox'], evaluate: evaluateAlignedWith, - }); - registerClause({ + }) + registry.register({ clauseKind: 'alignment.centeredWithin', version: 1, requiredFacts: ['subject.primaryBox', 'reference.primaryBox'], evaluate: evaluateCenteredWithin, - }); - registerClause({ + }) + registry.register({ clauseKind: 'size.atLeast', version: 1, requiredFacts: ['subject.primaryBox'], evaluate: evaluateAtLeast, - }); - registerClause({ + }) + registry.register({ clauseKind: 'size.atMost', version: 1, requiredFacts: ['subject.primaryBox'], evaluate: evaluateAtMost, - }); - registerClause({ + }) + registry.register({ clauseKind: 'size.between', version: 1, requiredFacts: ['subject.primaryBox'], evaluate: evaluateBetween, - }); - registerClause({ + }) + registry.register({ clauseKind: 'size.aspectRatio', version: 1, requiredFacts: ['subject.primaryBox'], evaluate: evaluateAspectRatio, - }); - registerClause({ + }) + registry.register({ clauseKind: 'topology.clippedBy', version: 1, requiredFacts: ['subject.clipChain', 'reference.clipChain'], evaluate: evaluateClippedBy, - }); - registerClause({ + }) + registry.register({ clauseKind: 'topology.attachedToScrollContainer', version: 1, requiredFacts: ['topology.scrollContainerOf'], evaluate: evaluateAttachedToScrollContainer, - }); - registerClause({ + }) + registry.register({ clauseKind: 'topology.inStackingContext', version: 1, requiredFacts: ['topology.stackingContextOf'], evaluate: evaluateInStackingContext, - }); + }) +} + +/** Create a fresh ClauseRegistry with all built-in clause evaluators pre-registered. */ +export function createDefaultClauseRegistry(): ClauseRegistry { + const registry = new ClauseRegistry() + populateDefaultClauses(registry) + return registry +} + +/** @deprecated Use createDefaultClauseRegistry() or explicit ClauseRegistry injection. */ +export function registerDefaultClauses(): void { + populateDefaultClauses(globalClauseRegistry) } /** @@ -176,7 +195,7 @@ export function evaluate( clauses: ClauseDescriptor[], options: EvaluationOptions = {}, ): EvaluationOutput { - const registry = options.registry ?? globalClauseRegistry; + const registry = options.registry ?? createDefaultClauseRegistry(); // Reset per-evaluation transform caches so visual rects are recomputed // once per subject per evaluation batch. clearEvaluationCache(world); diff --git a/packages/imhotep-solver/src/index.ts b/packages/imhotep-solver/src/index.ts index 2a00fc2..5f288a6 100644 --- a/packages/imhotep-solver/src/index.ts +++ b/packages/imhotep-solver/src/index.ts @@ -66,6 +66,8 @@ export { evaluate, collectRequiredFacts, registerDefaultClauses, + createDefaultClauseRegistry, + populateDefaultClauses, type EvaluationOptions, type EvaluationOutput, } from './engine.js'; @@ -78,6 +80,8 @@ export { getPredicateDescriptor, getRequiredFactsForPredicate, registerDefaultPredicates, + createDefaultPredicateRegistry, + populateDefaultPredicates, BUILTIN_PREDICATES, type PredicateDescriptor, type PredicateEvaluator, diff --git a/packages/imhotep-solver/src/logic-engine.ts b/packages/imhotep-solver/src/logic-engine.ts index ac4822e..7f1ad6e 100644 --- a/packages/imhotep-solver/src/logic-engine.ts +++ b/packages/imhotep-solver/src/logic-engine.ts @@ -37,7 +37,7 @@ import { getPredicateEvaluator, registerDefaultPredicates, PredicateRegistry, - globalPredicateRegistry, + createDefaultPredicateRegistry, } from './predicates.js'; import { @@ -906,7 +906,7 @@ export function evaluateLogic(input: LogicEngineInput): DeterministicSceneEvalua proofs: [], proofCounter: 0, formulaCounter: 0, - predicateRegistry: options.predicateRegistry ?? globalPredicateRegistry, + predicateRegistry: options.predicateRegistry ?? createDefaultPredicateRegistry(), }; addTrace(state, 'evaluate-logic-start'); diff --git a/packages/imhotep-solver/src/predicates.ts b/packages/imhotep-solver/src/predicates.ts index 1940233..7b91925 100644 --- a/packages/imhotep-solver/src/predicates.ts +++ b/packages/imhotep-solver/src/predicates.ts @@ -1200,45 +1200,56 @@ export const hasGapPredicate: PredicateEvaluator = { /** Sentinel registered to detect if defaults were already installed. */ const DEFAULT_SENTINEL = '__imhotep_defaults_registered__' -export function registerDefaultPredicates(): void { - if (globalPredicateRegistry.get(DEFAULT_SENTINEL)) return - // Register sentinel first so partial failures don't cause infinite loops. - globalPredicateRegistry.register({ +/** Populate a PredicateRegistry with all 33 built-in predicates (idempotent). */ +export function populateDefaultPredicates(registry: PredicateRegistry): void { + if (registry.get(DEFAULT_SENTINEL)) return + registry.register({ descriptor: { name: DEFAULT_SENTINEL, arity: 0, domains: [], requiredFacts: [] }, evaluateTuple: () => ({ truth: 'indeterminate' }), }) - registerPredicate(widthPredicate); - registerPredicate(heightPredicate); - registerPredicate(abovePredicate); - registerPredicate(belowPredicate); - registerPredicate(leftOfPredicate); - registerPredicate(rightOfPredicate); - registerPredicate(insidePredicate); - registerPredicate(containsPredicate); - registerPredicate(overlapsPredicate); - registerPredicate(alignedWithPredicate); - registerPredicate(centeredWithinPredicate); - registerPredicate(atLeastPredicate); - registerPredicate(atMostPredicate); - registerPredicate(betweenPredicate); - registerPredicate(clippedByPredicate); - registerPredicate(attachedToScrollContainerPredicate); - registerPredicate(escapeClippingChainOfPredicate); - registerPredicate(aspectRatioPredicate); - registerPredicate(inStackingContextPredicate); - registerPredicate(separatedFromPredicate); - registerPredicate(leftAlignedWithPredicate); - registerPredicate(rightAlignedWithPredicate); - registerPredicate(topAlignedWithPredicate); - registerPredicate(bottomAlignedWithPredicate); - registerPredicate(intersectsPredicate); - registerPredicate(touchesPredicate); - registerPredicate(hasGapPredicate); - registerPredicate(besidePredicate); - registerPredicate(nextToPredicate); - registerPredicate(adjacentPredicate); - registerPredicate(touchingPredicate); - registerPredicate(nearPredicate); - registerPredicate(underPredicate); - registerPredicate(withinPredicate); + registry.register(widthPredicate) + registry.register(heightPredicate) + registry.register(abovePredicate) + registry.register(belowPredicate) + registry.register(leftOfPredicate) + registry.register(rightOfPredicate) + registry.register(insidePredicate) + registry.register(containsPredicate) + registry.register(overlapsPredicate) + registry.register(alignedWithPredicate) + registry.register(centeredWithinPredicate) + registry.register(atLeastPredicate) + registry.register(atMostPredicate) + registry.register(betweenPredicate) + registry.register(clippedByPredicate) + registry.register(attachedToScrollContainerPredicate) + registry.register(escapeClippingChainOfPredicate) + registry.register(aspectRatioPredicate) + registry.register(inStackingContextPredicate) + registry.register(separatedFromPredicate) + registry.register(leftAlignedWithPredicate) + registry.register(rightAlignedWithPredicate) + registry.register(topAlignedWithPredicate) + registry.register(bottomAlignedWithPredicate) + registry.register(intersectsPredicate) + registry.register(touchesPredicate) + registry.register(hasGapPredicate) + registry.register(besidePredicate) + registry.register(nextToPredicate) + registry.register(adjacentPredicate) + registry.register(touchingPredicate) + registry.register(nearPredicate) + registry.register(underPredicate) + registry.register(withinPredicate) +} + +/** Create a fresh PredicateRegistry with all 33 built-in predicates pre-registered. */ +export function createDefaultPredicateRegistry(): PredicateRegistry { + const registry = new PredicateRegistry() + populateDefaultPredicates(registry) + return registry +} + +export function registerDefaultPredicates(): void { + populateDefaultPredicates(globalPredicateRegistry) } diff --git a/packages/imhotep-solver/src/registry.ts b/packages/imhotep-solver/src/registry.ts index ebd254f..ab111cb 100644 --- a/packages/imhotep-solver/src/registry.ts +++ b/packages/imhotep-solver/src/registry.ts @@ -297,7 +297,7 @@ export class ClauseRegistry { /** @deprecated Use explicit ClauseRegistry injection via EvaluationOptions.registry. */ export const globalClauseRegistry = new ClauseRegistry(); -/** Register a clause family so the engine can route evaluation. */ +/** Register a clause family in the global registry. @deprecated Use ClauseRegistry.register directly. */ export function registerClause(entry: ClauseEntry): void { globalClauseRegistry.register(entry); }