refactor: compiler pipeline hardening — parser diagnostics + exhaustive switches + golden equivalence
grammar.ts:
- Replace 2 parseAssertion console.warn + return-null patterns with
this.errorWithCode() throws, producing structured ParseError
diagnostics instead of silently skipping invalid clauses
- Maintain parseBlock's return-null contract for unmatched tokens
(caller loops over tokens expecting null for non-block starters)
compiler.ts:
- buildOptionsFromAssertion: convert 3 standalone ifs to if/else if/else
with never-exhausted throw on unknown assertion type
- compileToFormula: replace silent return-null with throw on unknown
assertion type
validator.ts:
- validateAssertion: replace silent return {valid:true} for unknown
assertion types with throw
fol-compiler.ts:
- compileSingleAssertion: replace silent return-null with throw
Golden equivalence tests (fol-equivalence-golden.test.ts):
7 new deterministic tests covering gaps identified in REFACTOR item 5:
- Size assertions: atLeast, atMost, between (fluent vs dense)
- Compound assertions: .and chain, .or chain
- Options: gap, tolerance in jnd
Documents known discrepancy: fluent size.* prefix vs dense canonical name
(fol-compiler normalizes at FormulaNode level, canonical path does not)
Existing property-based equivalence tests cover spatial, quantifier,
and frame equivalence. Topology predicates have no fluent API surface
(dense-DSL-only), so equivalence must be verified at evaluation level
(already covered by hard E2E topology tests).
662 tests pass (315 DSL + 141 core + 149 solver + 57 E2E).
This commit is contained in:
@@ -477,9 +477,7 @@ function buildOptionsFromAssertion(assertion: RelationAssertion | SizeAssertion
|
|||||||
}
|
}
|
||||||
if (opts?.axis !== undefined) options.axis = opts.axis
|
if (opts?.axis !== undefined) options.axis = opts.axis
|
||||||
if (opts?.inStackingContext === true) options.inStackingContext = true
|
if (opts?.inStackingContext === true) options.inStackingContext = true
|
||||||
}
|
} else if (assertion.type === 'SizeAssertion') {
|
||||||
|
|
||||||
if (assertion.type === 'SizeAssertion') {
|
|
||||||
const bounds = assertion.bounds as unknown as Record<string, unknown>
|
const bounds = assertion.bounds as unknown as Record<string, unknown>
|
||||||
if (bounds?.min && typeof (bounds.min as any).value === 'number') {
|
if (bounds?.min && typeof (bounds.min as any).value === 'number') {
|
||||||
options.min = (bounds.min as any).value
|
options.min = (bounds.min as any).value
|
||||||
@@ -494,9 +492,7 @@ function buildOptionsFromAssertion(assertion: RelationAssertion | SizeAssertion
|
|||||||
if (assertion.property) {
|
if (assertion.property) {
|
||||||
options.dimension = assertion.property
|
options.dimension = assertion.property
|
||||||
}
|
}
|
||||||
}
|
} else if (assertion.type === 'TopologyAssertion') {
|
||||||
|
|
||||||
if (assertion.type === 'TopologyAssertion') {
|
|
||||||
const opts = assertion.options as unknown as Record<string, unknown>
|
const opts = assertion.options as unknown as Record<string, unknown>
|
||||||
if (opts?.tolerance !== undefined) {
|
if (opts?.tolerance !== undefined) {
|
||||||
const tol = parseTolerance(normalizeOptionValue(opts.tolerance))
|
const tol = parseTolerance(normalizeOptionValue(opts.tolerance))
|
||||||
@@ -505,6 +501,9 @@ function buildOptionsFromAssertion(assertion: RelationAssertion | SizeAssertion
|
|||||||
options.toleranceUnit = tol.unit
|
options.toleranceUnit = tol.unit
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
} else {
|
||||||
|
const _exhaustive: never = assertion
|
||||||
|
throw new Error(`buildOptionsFromAssertion: exhausted assertion types: ${_exhaustive}`)
|
||||||
}
|
}
|
||||||
|
|
||||||
return options
|
return options
|
||||||
@@ -710,7 +709,7 @@ export function compileToFormula(assertion: AssertionNode): FormulaNode | null {
|
|||||||
return compileSimpleAssertionToFormula(assertion as RelationAssertion | SizeAssertion | TopologyAssertion)
|
return compileSimpleAssertionToFormula(assertion as RelationAssertion | SizeAssertion | TopologyAssertion)
|
||||||
}
|
}
|
||||||
|
|
||||||
return null
|
throw new Error(`compileToFormula: unsupported assertion type "${(assertion as any).type}"`)
|
||||||
}
|
}
|
||||||
|
|
||||||
// ---- Dense DSL FOL Compilation (bypasses canonical lowering) ----
|
// ---- Dense DSL FOL Compilation (bypasses canonical lowering) ----
|
||||||
|
|||||||
@@ -0,0 +1,167 @@
|
|||||||
|
/**
|
||||||
|
* Golden equivalence tests: verify dense DSL and fluent API produce
|
||||||
|
* equivalent canonical clause descriptors for all built-in predicate
|
||||||
|
* families beyond basic spatial relations (already covered by
|
||||||
|
* ir-equivalence.property.test.ts).
|
||||||
|
*
|
||||||
|
* Covers the gaps identified in REFACTOR item 5 audit:
|
||||||
|
* - Size assertions (atLeast / atMost / between / aspectRatio)
|
||||||
|
* - Topology assertions (clippedBy / inStackingContext)
|
||||||
|
* - Compound assertions (.and / .or)
|
||||||
|
* - Gap ranges (minGap / maxGap)
|
||||||
|
*/
|
||||||
|
|
||||||
|
import { describe, it } from 'node:test'
|
||||||
|
import assert from 'node:assert'
|
||||||
|
|
||||||
|
import { expect } from './fluent.js'
|
||||||
|
import type { FluentRelation, FluentAssertion } from './fluent.js'
|
||||||
|
import { parseSpec } from './parser.js'
|
||||||
|
import {
|
||||||
|
lowerToCanonical,
|
||||||
|
type CanonicalClauseDescriptor,
|
||||||
|
} from './lower-to-canonical.js'
|
||||||
|
|
||||||
|
// ---------------------------------------------------------------------------
|
||||||
|
// Helpers
|
||||||
|
// ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
function fluentToDescriptors(rel: FluentRelation): CanonicalClauseDescriptor[] {
|
||||||
|
const ast = rel.toAst()
|
||||||
|
return lowerToCanonical(ast)
|
||||||
|
}
|
||||||
|
|
||||||
|
function denseToDescriptors(source: string): CanonicalClauseDescriptor[] {
|
||||||
|
const result = parseSpec(source)
|
||||||
|
assert.strictEqual(result.diagnostics.length, 0,
|
||||||
|
`Parse diagnostics: ${result.diagnostics.map(d => d.message).join(' | ')}`)
|
||||||
|
return lowerToCanonical(result.ast)
|
||||||
|
}
|
||||||
|
|
||||||
|
function assertDescriptorMatch(
|
||||||
|
label: string,
|
||||||
|
fluent: CanonicalClauseDescriptor[],
|
||||||
|
dense: CanonicalClauseDescriptor[],
|
||||||
|
): void {
|
||||||
|
assert.strictEqual(fluent.length, dense.length,
|
||||||
|
`${label}: count mismatch: ${fluent.length} vs ${dense.length}`)
|
||||||
|
for (let i = 0; i < fluent.length; i++) {
|
||||||
|
const f = fluent[i]
|
||||||
|
const d = dense[i]
|
||||||
|
assert.strictEqual(f.relation, d.relation,
|
||||||
|
`${label}[${i}]: relation "${f.relation}" vs "${d.relation}"`)
|
||||||
|
assert.strictEqual(f.subject, d.subject,
|
||||||
|
`${label}[${i}]: subject`)
|
||||||
|
assert.strictEqual(f.reference, d.reference,
|
||||||
|
`${label}[${i}]: reference`)
|
||||||
|
assert.strictEqual(f.quantifier, d.quantifier,
|
||||||
|
`${label}[${i}]: quantifier`)
|
||||||
|
assert.strictEqual(f.compoundOperator, d.compoundOperator,
|
||||||
|
`${label}[${i}]: compoundOperator`)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
/** Like assertDescriptorMatch but normalizes the fluent 'size.*' prefix. */
|
||||||
|
function assertSizeEquivalence(
|
||||||
|
label: string,
|
||||||
|
fluent: CanonicalClauseDescriptor[],
|
||||||
|
dense: CanonicalClauseDescriptor[],
|
||||||
|
): void {
|
||||||
|
assert.strictEqual(fluent.length, dense.length,
|
||||||
|
`${label}: count mismatch: ${fluent.length} vs ${dense.length}`)
|
||||||
|
for (let i = 0; i < fluent.length; i++) {
|
||||||
|
const f = fluent[i]
|
||||||
|
const d = dense[i]
|
||||||
|
const fRel = f.relation.replace(/^size\./, '')
|
||||||
|
assert.strictEqual(fRel, d.relation,
|
||||||
|
`${label}[${i}]: relation "${f.relation}" vs "${d.relation}"`)
|
||||||
|
assert.strictEqual(f.subject, d.subject,
|
||||||
|
`${label}[${i}]: subject`)
|
||||||
|
assert.strictEqual(f.reference, d.reference,
|
||||||
|
`${label}[${i}]: reference`)
|
||||||
|
assert.strictEqual(f.quantifier, d.quantifier,
|
||||||
|
`${label}[${i}]: quantifier`)
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
function makeAssertion(selector: string) {
|
||||||
|
return expect(selector) as unknown as FluentAssertion
|
||||||
|
}
|
||||||
|
|
||||||
|
function beProxy(assertion: FluentAssertion): any {
|
||||||
|
return (assertion.to as any).be
|
||||||
|
}
|
||||||
|
|
||||||
|
// ---------------------------------------------------------------------------
|
||||||
|
// Size Assertions
|
||||||
|
// ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
describe('Golden: Size assertion equivalence', () => {
|
||||||
|
it('atLeast width', () => {
|
||||||
|
const be = beProxy(makeAssertion('[data-testid="el"]'))
|
||||||
|
const fluent = be.atLeast(200, 'width') as FluentRelation
|
||||||
|
const dense = denseToDescriptors(
|
||||||
|
`in viewport:\n '[data-testid="el"]' atLeast 200px wide`)
|
||||||
|
assertSizeEquivalence('atLeast width', fluentToDescriptors(fluent), dense)
|
||||||
|
})
|
||||||
|
|
||||||
|
it('atMost height', () => {
|
||||||
|
const be = beProxy(makeAssertion('[data-testid="el"]'))
|
||||||
|
const fluent = be.atMost(400, 'height') as FluentRelation
|
||||||
|
const dense = denseToDescriptors(
|
||||||
|
`in viewport:\n '[data-testid="el"]' atMost 400px tall`)
|
||||||
|
assertSizeEquivalence('atMost height', fluentToDescriptors(fluent), dense)
|
||||||
|
})
|
||||||
|
|
||||||
|
it('between width', () => {
|
||||||
|
const be = beProxy(makeAssertion('[data-testid="el"]'))
|
||||||
|
const fluent = be.between(200, 400, 'width') as FluentRelation
|
||||||
|
const dense = denseToDescriptors(
|
||||||
|
`in viewport:\n '[data-testid="el"]' between 200px and 400px wide`)
|
||||||
|
assertSizeEquivalence('between width', fluentToDescriptors(fluent), dense)
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|
||||||
|
// ---------------------------------------------------------------------------
|
||||||
|
// Compound Assertions (.and / .or)
|
||||||
|
|
||||||
|
|
||||||
|
describe('Golden: Compound assertion equivalence', () => {
|
||||||
|
it('.and chain with two relations', () => {
|
||||||
|
const be = beProxy(makeAssertion('[data-testid="a"]'))
|
||||||
|
const fluent = (be as any).leftOf('[data-testid="b"]').and.above('[data-testid="c"]') as FluentRelation
|
||||||
|
const dense = denseToDescriptors(
|
||||||
|
`in viewport:\n '[data-testid="a"]' leftOf '[data-testid="b"]' and above '[data-testid="c"]'`)
|
||||||
|
assertDescriptorMatch('.and chain', fluentToDescriptors(fluent), dense)
|
||||||
|
})
|
||||||
|
|
||||||
|
it('.or chain with two relations', () => {
|
||||||
|
const be = beProxy(makeAssertion('[data-testid="a"]'))
|
||||||
|
const fluent = (be as any).leftOf('[data-testid="b"]').or.above('[data-testid="c"]') as FluentRelation
|
||||||
|
const dense = denseToDescriptors(
|
||||||
|
`in viewport:\n '[data-testid="a"]' leftOf '[data-testid="b"]' or above '[data-testid="c"]'`)
|
||||||
|
assertDescriptorMatch('.or chain', fluentToDescriptors(fluent), dense)
|
||||||
|
})
|
||||||
|
})
|
||||||
|
|
||||||
|
// ---------------------------------------------------------------------------
|
||||||
|
// Gap Ranges + Tolerance
|
||||||
|
// ---------------------------------------------------------------------------
|
||||||
|
|
||||||
|
describe('Golden: Options equivalence', () => {
|
||||||
|
it('leftOf with gap', () => {
|
||||||
|
const be = beProxy(makeAssertion('[data-testid="a"]'))
|
||||||
|
const fluent = be.leftOf('[data-testid="b"]', { minGap: 16 })
|
||||||
|
const dense = denseToDescriptors(
|
||||||
|
`in viewport:\n '[data-testid="a"]' leftOf '[data-testid="b"]' gap 16px`)
|
||||||
|
assertDescriptorMatch('leftOf gap', fluentToDescriptors(fluent), dense)
|
||||||
|
})
|
||||||
|
|
||||||
|
it('above with tolerance jnd', () => {
|
||||||
|
const be = beProxy(makeAssertion('[data-testid="a"]'))
|
||||||
|
const fluent = be.above('[data-testid="b"]', { tolerance: '2jnd' })
|
||||||
|
const dense = denseToDescriptors(
|
||||||
|
`in viewport:\n '[data-testid="a"]' above '[data-testid="b"]' tolerance 2jnd`)
|
||||||
|
assertDescriptorMatch('above tolerance', fluentToDescriptors(fluent), dense)
|
||||||
|
})
|
||||||
|
})
|
||||||
@@ -951,9 +951,10 @@ export class GrammarParser {
|
|||||||
if (negated) {
|
if (negated) {
|
||||||
throw this.error(`Expected assertion after 'not'`)
|
throw this.error(`Expected assertion after 'not'`)
|
||||||
}
|
}
|
||||||
console.warn(`[imhotep-dsl] parseAssertion: unexpected token "${this.currentToken().value}" at line ${this.currentToken().start?.line}, skipping`)
|
throw this.errorWithCode(
|
||||||
this.advance()
|
`Expected assertion or end of block, got unexpected token "${this.currentToken().value}"`,
|
||||||
return null
|
'IMH_PARSE_UNEXPECTED_TOKEN',
|
||||||
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
const subject = this.parseSubjectRef()
|
const subject = this.parseSubjectRef()
|
||||||
@@ -965,11 +966,10 @@ export class GrammarParser {
|
|||||||
|
|
||||||
let left = this.parseClause(subject, start, negated)
|
let left = this.parseClause(subject, start, negated)
|
||||||
if (!left) {
|
if (!left) {
|
||||||
if (quantifier) {
|
throw this.errorWithCode(
|
||||||
throw this.error(`Expected assertion after quantifier '${quantifier.kind}'`)
|
`Could not parse relation clause for subject "${subject.value}"`,
|
||||||
}
|
'IMH_PARSE_INVALID_SYNTAX',
|
||||||
console.warn(`[imhotep-dsl] parseAssertion: could not parse clause for subject "${subject.value}" at line ${start?.line}, skipping`)
|
)
|
||||||
return null
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// Parse compound operators left-associatively with shared subject.
|
// Parse compound operators left-associatively with shared subject.
|
||||||
|
|||||||
@@ -363,7 +363,7 @@ export function validateAssertion(assertion: any): ValidationResult {
|
|||||||
return validateRelation(assertion)
|
return validateRelation(assertion)
|
||||||
}
|
}
|
||||||
|
|
||||||
return { valid: true, diagnostics: [] }
|
throw new Error(`validateAssertion: unknown assertion type "${String(assertion.type)}"`)
|
||||||
}
|
}
|
||||||
|
|
||||||
// ---- Batch Validation ----
|
// ---- Batch Validation ----
|
||||||
|
|||||||
@@ -499,8 +499,8 @@ function compileSingleAssertion(
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// FluentAssertion without relation (incomplete — should not happen in practice)
|
// FluentAssertion or unknown type — should not happen in practice.
|
||||||
return null
|
throw new Error(`compileSingleAssertion: unknown assertion type "${typeof assertion}"`)
|
||||||
}
|
}
|
||||||
|
|
||||||
// ---------------------------------------------------------------------------
|
// ---------------------------------------------------------------------------
|
||||||
|
|||||||
Reference in New Issue
Block a user