refactor: eliminate remaining hardcoded predicate-name dispatch
Extraction.ts (3 fixes):
- Replace 2 'inStackingContext' string checks with isVariableArityPredicate()
- Replace 7-name diagnostic formatting if/else with spec-driven
getPredicateSpec() checks (isDirectional → gap message,
isSize → threshold hint, else generic)
Grammar.ts: Replace 8 hardcoded parser routing checks
(atLeast/atMost/aspectRatio/between/clippedBy/attachedToScrollContainer/
escapeClippingChainOf/inStackingContext) with SIZE_PREDICATE_NAMES and
TOPOLOGY_PREDICATE_NAMES Sets derived from spec table.
Pipeline.ts: Replace 15-entry CODE_TO_CLAUSE_KIND map with runtime
generation from PREDICATE_SPECS. Prefix derived from spec.isSize
('size.*') / validOptions.includes('axis') ('alignment.*') /
else ('relation.*'). Manual override for aspectRatio code 15.
Proofs.ts: Replace 11-case switch(kind) with 5 spec-driven if/else
branches categorized by validOptions presence (hasGap→directional,
hasAxis→alignment) + 2 specific name checks (inside overflow,
aspectRatio ratio). 11 predicate names → 0 hardcoded.
Lexer.ts: Export KEYWORDS map for conformance testing.
Conformance tests:
- Solver: every BUILTIN_PREDICATES entry matches its PREDICATE_SPECS
counterpart; every spec name (incl. aliases) has a registered
evaluator with matching descriptor (2 tests)
- DSL: every predicate name from collectAllPredicateNames() appears
in the lexer KEYWORDS table (1 test)
598 SDK + 3 conformance + 57 E2E = 658 tests pass.
This commit is contained in:
@@ -106,6 +106,7 @@ import type {
|
||||
} from './diagnostics.js'
|
||||
|
||||
import { createDiagnostic } from './diagnostics.js'
|
||||
import { PREDICATE_SPECS } from './predicate-specs.js'
|
||||
|
||||
// Adapter interfaces - implementations injected at runtime
|
||||
interface ExtractorRequest {
|
||||
@@ -433,25 +434,23 @@ export interface PipelineResult {
|
||||
|
||||
/**
|
||||
* Reverse mapping from execution IR clause type codes to solver clause kinds.
|
||||
* Mirrors the relationCodes table in imhotep-dsl/compiler.ts.
|
||||
* Built from the predicate spec table — any predicate with a relationCode
|
||||
* automatically gets a clause kind entry.
|
||||
*/
|
||||
const CODE_TO_CLAUSE_KIND: Record<number, string> = {
|
||||
1: 'relation.leftOf',
|
||||
2: 'relation.rightOf',
|
||||
3: 'relation.above',
|
||||
4: 'relation.below',
|
||||
5: 'alignment.alignedWith',
|
||||
6: 'alignment.leftAlignedWith',
|
||||
7: 'alignment.rightAlignedWith',
|
||||
8: 'alignment.topAlignedWith',
|
||||
9: 'alignment.bottomAlignedWith',
|
||||
10: 'alignment.centeredWithin',
|
||||
11: 'relation.inside',
|
||||
12: 'relation.contains',
|
||||
13: 'relation.overlaps',
|
||||
14: 'relation.separatedFrom',
|
||||
15: 'size.aspectRatio',
|
||||
const CODE_TO_CLAUSE_KIND: Record<number, string> = (() => {
|
||||
const map: Record<number, string> = {}
|
||||
for (const spec of PREDICATE_SPECS) {
|
||||
if (!spec.relationCode) continue
|
||||
let prefix = 'relation'
|
||||
if (spec.isSize) prefix = 'size'
|
||||
else if (spec.validOptions.includes('axis')) prefix = 'alignment'
|
||||
map[spec.relationCode] = `${prefix}.${spec.name}`
|
||||
}
|
||||
return map
|
||||
})()
|
||||
// DSL keyword 'aspectRatioBetween' uses code 15; maps to the canonical
|
||||
// 'aspectRatio' predicate which has no separate relationCode.
|
||||
CODE_TO_CLAUSE_KIND[15] = 'size.aspectRatio'
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Main Pipeline Orchestrator
|
||||
|
||||
@@ -6,6 +6,8 @@ import assert from 'node:assert'
|
||||
|
||||
import { parseSpec } from './parser.js'
|
||||
import { lowerToCanonical } from './lower-to-canonical.js'
|
||||
import { KEYWORDS } from './lexer.js'
|
||||
import { collectAllPredicateNames } from 'imhotep-core'
|
||||
|
||||
function parse(source: string) {
|
||||
return parseSpec(source)
|
||||
@@ -449,3 +451,20 @@ describe('inline state attachments', () => {
|
||||
assert.strictEqual(top.state.kind, 'expanded')
|
||||
})
|
||||
})
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Conformance: PREDICATE_SPECS ↔ Lexer KEYWORDS sync
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe('PredicateSpec ↔ Lexer keyword conformance', () => {
|
||||
it('every predicate name from PREDICATE_SPECS has a lexer keyword entry', () => {
|
||||
const allNames = collectAllPredicateNames()
|
||||
const missing: string[] = []
|
||||
for (const name of allNames) {
|
||||
if (!(name in KEYWORDS)) {
|
||||
missing.push(name)
|
||||
}
|
||||
}
|
||||
assert.deepStrictEqual(missing, [], `Predicate names missing from lexer KEYWORDS: ${missing.join(', ')}`)
|
||||
})
|
||||
})
|
||||
|
||||
@@ -27,10 +27,14 @@ import type {
|
||||
ToleranceLiteralNode,
|
||||
} from 'imhotep-core'
|
||||
|
||||
import { isPredicateName, collectSpatialPredicateNames } from 'imhotep-core'
|
||||
import { isPredicateName, collectSpatialPredicateNames, collectSizePredicateNames, collectTopologyPredicateNames } from 'imhotep-core'
|
||||
|
||||
import type { Token } from './lexer.js'
|
||||
|
||||
// Cached predicate name sets derived from the spec table.
|
||||
const SIZE_PREDICATE_NAMES = new Set(collectSizePredicateNames())
|
||||
const TOPOLOGY_PREDICATE_NAMES = new Set(collectTopologyPredicateNames())
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Extended TokenKind for grammar checks
|
||||
// ---------------------------------------------------------------------------
|
||||
@@ -1002,14 +1006,13 @@ export class GrammarParser {
|
||||
private parseClause(subject: SelectorRef, start: Point, negated: boolean): any | null {
|
||||
let assertion: any
|
||||
|
||||
// Size assertion
|
||||
if (this.check('atLeast') || this.check('atMost') || this.check('aspectRatio') || this.check('between')) {
|
||||
// Dispatch assertion type based on predicate category from spec table.
|
||||
const kind = this.currentToken().kind as string
|
||||
if (SIZE_PREDICATE_NAMES.has(kind)) {
|
||||
assertion = this.parseSizeAssertion(subject, start)
|
||||
} else if (this.check('clippedBy') || this.check('attachedToScrollContainer') || this.check('escapeClippingChainOf') || this.check('inStackingContext')) {
|
||||
// Topology assertion
|
||||
} else if (TOPOLOGY_PREDICATE_NAMES.has(kind)) {
|
||||
assertion = this.parseTopologyAssertion(subject, start)
|
||||
} else {
|
||||
// Relation assertion
|
||||
assertion = this.parseRelationAssertion(subject, start)
|
||||
}
|
||||
|
||||
|
||||
@@ -143,7 +143,7 @@ export interface Token {
|
||||
// Keywords Map
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
const KEYWORDS: Record<string, TokenKind> = {
|
||||
export const KEYWORDS: Record<string, TokenKind> = {
|
||||
in: 'in',
|
||||
across: 'across',
|
||||
always: 'always',
|
||||
|
||||
@@ -56,6 +56,8 @@ import {
|
||||
getPredicateDiagnosticCode,
|
||||
getPredicateDecomposition,
|
||||
isUnaryPredicate,
|
||||
isVariableArityPredicate,
|
||||
getPredicateSpec,
|
||||
} from 'imhotep-core'
|
||||
import { buildGeometryWorld } from './world-builder.js'
|
||||
import {
|
||||
@@ -1206,7 +1208,7 @@ export function compileCanonicalClauseToFormula(clause: CanonicalClauseDescripto
|
||||
}
|
||||
} else {
|
||||
const unaryPredicate = isUnaryPredicate(clause.relation)
|
||||
&& !(clause.relation === 'inStackingContext' && clause.reference)
|
||||
&& !(isVariableArityPredicate(clause.relation) && clause.reference)
|
||||
|
||||
body = {
|
||||
type: 'FormulaNode',
|
||||
@@ -1251,7 +1253,7 @@ export function compileCanonicalClauseToFormula(clause: CanonicalClauseDescripto
|
||||
}
|
||||
|
||||
const isUnary = isUnaryPredicate(clause.relation)
|
||||
&& !(clause.relation === 'inStackingContext' && clause.reference)
|
||||
&& !(isVariableArityPredicate(clause.relation) && clause.reference)
|
||||
|
||||
if (isUnary) {
|
||||
return {
|
||||
@@ -1645,19 +1647,12 @@ export function mapFolDiagnostic(
|
||||
: 'min'
|
||||
const expectedGap = gapKind === 'min' ? (minGap ?? 0) : (maxGap ?? 0)
|
||||
const boundDescription = gapKind === 'min' ? 'minimum required gap' : 'maximum required gap'
|
||||
if (predicateName === 'leftOf' && observedGap !== undefined) {
|
||||
message = `leftOf assertion failed: measured gap is ${observedGap.toFixed(0)}px, but ${boundDescription} is ${expectedGap}px.`
|
||||
const spec = predicateName ? getPredicateSpec(predicateName) : undefined
|
||||
const isDirectional = spec?.validOptions.includes('minGap') ?? false
|
||||
if (isDirectional && observedGap !== undefined) {
|
||||
message = `${predicateName} assertion failed: measured gap is ${observedGap.toFixed(0)}px, but ${boundDescription} is ${expectedGap}px.`
|
||||
fixHints.push(`The measured gap is ${observedGap.toFixed(0)}px. Consider ${gapKind === 'min' ? 'lowering minGap' : 'increasing maxGap'} or checking element positions.`)
|
||||
} else if (predicateName === 'above' && observedGap !== undefined) {
|
||||
message = `above assertion failed: measured gap is ${observedGap.toFixed(0)}px, but ${boundDescription} is ${expectedGap}px.`
|
||||
fixHints.push(`The measured gap is ${observedGap.toFixed(0)}px. Consider ${gapKind === 'min' ? 'lowering minGap' : 'increasing maxGap'} or checking element positions.`)
|
||||
} else if (predicateName === 'below' && observedGap !== undefined) {
|
||||
message = `below assertion failed: measured gap is ${observedGap.toFixed(0)}px, but ${boundDescription} is ${expectedGap}px.`
|
||||
fixHints.push(`The measured gap is ${observedGap.toFixed(0)}px. Consider ${gapKind === 'min' ? 'lowering minGap' : 'increasing maxGap'} or checking element positions.`)
|
||||
} else if (predicateName === 'rightOf' && observedGap !== undefined) {
|
||||
message = `rightOf assertion failed: measured gap is ${observedGap.toFixed(0)}px, but ${boundDescription} is ${expectedGap}px.`
|
||||
fixHints.push(`The measured gap is ${observedGap.toFixed(0)}px. Consider ${gapKind === 'min' ? 'lowering minGap' : 'increasing maxGap'} or checking element positions.`)
|
||||
} else if (predicateName === 'atLeast' || predicateName === 'atMost' || predicateName === 'between') {
|
||||
} else if (spec?.isSize) {
|
||||
fixHints.push(`Check the expected size threshold and the actual element dimensions using ui.extract(selector).`)
|
||||
} else {
|
||||
fixHints.push(`Verify the expected layout and consider adjusting thresholds.`)
|
||||
|
||||
@@ -17,8 +17,10 @@ import {
|
||||
clearPredicateRegistry,
|
||||
registerDefaultPredicates,
|
||||
getPredicateEvaluator,
|
||||
BUILTIN_PREDICATES,
|
||||
type GeometryWorld,
|
||||
} from './index.js';
|
||||
import { PREDICATE_SPECS, getAllPredicateSpecs, getPredicateSpec } from 'imhotep-core';
|
||||
|
||||
// --- Test helpers ------------------------------------------------------------
|
||||
|
||||
@@ -683,4 +685,55 @@ describe('spatial alias predicates', () => {
|
||||
assert.ok(Math.abs((result.metrics?.gap ?? 0) - Math.sqrt(200)) < 0.0001);
|
||||
});
|
||||
});
|
||||
|
||||
// ---------------------------------------------------------------------------
|
||||
// Conformance: PREDICATE_SPECS ↔ BUILTIN_PREDICATES sync
|
||||
// ---------------------------------------------------------------------------
|
||||
|
||||
describe('PredicateSpec ↔ BUILTIN_PREDICATES conformance', () => {
|
||||
it('every BUILTIN_PREDICATES entry has a matching PREDICATE_SPECS entry', () => {
|
||||
for (const bp of BUILTIN_PREDICATES) {
|
||||
const spec = getPredicateSpec(bp.name)
|
||||
assert.ok(spec, `BUILTIN_PREDICATES entry "${bp.name}" not found in PREDICATE_SPECS`)
|
||||
assert.strictEqual(
|
||||
spec.arity === 'variable' ? 2 : spec.arity,
|
||||
bp.arity,
|
||||
`Arity mismatch for "${bp.name}": PREDICATE_SPECS has ${spec.arity}, BUILTIN_PREDICATES has ${bp.arity}`,
|
||||
)
|
||||
for (const fact of bp.requiredFacts) {
|
||||
assert.ok(
|
||||
spec.requiredFacts.includes(fact),
|
||||
`Required fact "${fact}" for "${bp.name}" is in BUILTIN_PREDICATES but not in PREDICATE_SPECS`,
|
||||
)
|
||||
}
|
||||
}
|
||||
})
|
||||
|
||||
it('every registered predicate has an evaluator with matching descriptor', () => {
|
||||
registerDefaultPredicates()
|
||||
// Collect all predicate names from spec table (including aliases as separate evaluator names).
|
||||
const specNames = new Set<string>()
|
||||
for (const spec of PREDICATE_SPECS) {
|
||||
specNames.add(spec.name)
|
||||
for (const alias of spec.aliases) specNames.add(alias)
|
||||
}
|
||||
for (const name of specNames) {
|
||||
const evaluator = getPredicateEvaluator(name)
|
||||
assert.ok(evaluator, `No evaluator registered for predicate "${name}" from PREDICATE_SPECS`)
|
||||
const spec = getPredicateSpec(name)
|
||||
if (spec) {
|
||||
const desc = evaluator.descriptor
|
||||
assert.strictEqual(desc.name, name, `Evaluator name mismatch for "${name}"`)
|
||||
const expectedArity = spec.arity === 'variable' ? 2 : spec.arity
|
||||
assert.strictEqual(desc.arity, expectedArity, `Evaluator arity mismatch for "${name}"`)
|
||||
for (const fact of spec.requiredFacts) {
|
||||
assert.ok(
|
||||
desc.requiredFacts.includes(fact),
|
||||
`Required fact "${fact}" for "${name}" is in PREDICATE_SPECS but missing from evaluator descriptor`,
|
||||
)
|
||||
}
|
||||
}
|
||||
}
|
||||
})
|
||||
})
|
||||
});
|
||||
|
||||
@@ -16,6 +16,8 @@ import {
|
||||
type GeometryWorld,
|
||||
} from './registry.js';
|
||||
|
||||
import { getPredicateSpec } from 'imhotep-core'
|
||||
|
||||
let proofCounter = 0;
|
||||
|
||||
function nextProofId(): string {
|
||||
@@ -49,11 +51,14 @@ function buildFailedPredicate(
|
||||
relationKind: kind,
|
||||
};
|
||||
|
||||
switch (kind) {
|
||||
case 'leftOf':
|
||||
case 'rightOf':
|
||||
case 'above':
|
||||
case 'below': {
|
||||
const spec = getPredicateSpec(kind)
|
||||
// Determine diagnostic shape family from spec metadata.
|
||||
const hasGap = spec?.validOptions.includes('minGap') && spec?.validOptions.includes('maxGap')
|
||||
const hasAxis = spec?.validOptions.includes('axis')
|
||||
const isSize = spec?.isSize
|
||||
|
||||
// --- Directional gap (leftOf / rightOf / above / below) ---
|
||||
if (hasGap) {
|
||||
const gap = metrics.observedGap ?? metrics.gap ?? 0;
|
||||
const min = metrics.minGap ?? 0;
|
||||
const max = metrics.maxGap ?? Infinity;
|
||||
@@ -86,7 +91,8 @@ function buildFailedPredicate(
|
||||
};
|
||||
}
|
||||
|
||||
case 'inside': {
|
||||
// --- Containment / inside (unique overflow metrics) ---
|
||||
if (kind === 'inside') {
|
||||
const overflowLeft = metrics.overflowLeft ?? 0;
|
||||
const overflowTop = metrics.overflowTop ?? 0;
|
||||
const overflowRight = metrics.overflowRight ?? 0;
|
||||
@@ -123,9 +129,8 @@ function buildFailedPredicate(
|
||||
};
|
||||
}
|
||||
|
||||
case 'atLeast':
|
||||
case 'atMost':
|
||||
case 'between': {
|
||||
// --- Size threshold (atLeast / atMost / between) ---
|
||||
if (isSize && kind !== 'aspectRatio') {
|
||||
const observed = metrics.observed ?? metrics.value ?? 0;
|
||||
const min = metrics.min ?? -Infinity;
|
||||
const max = metrics.max ?? Infinity;
|
||||
@@ -140,7 +145,8 @@ function buildFailedPredicate(
|
||||
};
|
||||
}
|
||||
|
||||
case 'aspectRatio': {
|
||||
// --- Aspect ratio ---
|
||||
if (kind === 'aspectRatio') {
|
||||
const observed = metrics.observed ?? 0;
|
||||
const minRatio = metrics.minRatio ?? -Infinity;
|
||||
const maxRatio = metrics.maxRatio ?? Infinity;
|
||||
@@ -155,8 +161,8 @@ function buildFailedPredicate(
|
||||
};
|
||||
}
|
||||
|
||||
case 'alignedWith':
|
||||
case 'centeredWithin': {
|
||||
// --- Alignment (alignedWith / centeredWithin) ---
|
||||
if (hasAxis || kind === 'centeredWithin') {
|
||||
const delta = metrics.delta ?? metrics.deltaX ?? metrics.deltaY ?? 0;
|
||||
const tolerance = metrics.tolerance ?? 0;
|
||||
return {
|
||||
@@ -169,7 +175,6 @@ function buildFailedPredicate(
|
||||
};
|
||||
}
|
||||
|
||||
default:
|
||||
// Fallback to generic synthesis from the first two numeric metrics.
|
||||
const fallback = synthesizeGenericFailedPredicate(metrics);
|
||||
if (fallback) {
|
||||
@@ -177,7 +182,6 @@ function buildFailedPredicate(
|
||||
}
|
||||
return fallback;
|
||||
}
|
||||
}
|
||||
|
||||
function synthesizeGenericFailedPredicate(
|
||||
metrics: Record<string, number>,
|
||||
|
||||
Reference in New Issue
Block a user