Re-implement branch disabling

Using the new infered "isNullable" property

packages/core/source/AST/graph.ts

@@ -74,7 +74,7 @@ function buildDependenciesGraph(rulesDeps: RulesDependencies) {
 type RawRules = Parameters<typeof parsePublicodes>[0]
 
 export function cyclesInDependenciesGraph(rawRules: RawRules): GraphCycles {
-	const parsedRules = parsePublicodes(rawRules)
+	const { parsedRules } = parsePublicodes(rawRules)
 	const rulesDependencies = buildRulesDependencies(parsedRules)
 	const dependenciesGraph = buildDependenciesGraph(rulesDependencies)
 	const cycles = findCycles(dependenciesGraph)
@@ -131,7 +131,7 @@ export function squashCycle(
 export function cyclicDependencies(
 	rawRules: RawRules
 ): [GraphCycles, string[]] {
-	const parsedRules = parsePublicodes(rawRules)
+	const { parsedRules } = parsePublicodes(rawRules)
 	const rulesDependencies = buildRulesDependencies(parsedRules)
 	const dependenciesGraph = buildDependenciesGraph(rulesDependencies)
 	const cycles = findCycles(dependenciesGraph)

packages/core/source/AST/index.ts

@@ -188,7 +188,6 @@ const traverseRuleNode: TraverseFunction<'rule'> = (fn, node) => ({
 		Object.entries(node.suggestions).map(([key, value]) => [key, fn(value)])
 	),
 	explanation: {
-		parent: node.explanation.parent && fn(node.explanation.parent),
 		valeur: fn(node.explanation.valeur),
 	},
 })

packages/core/source/index.ts

@@ -2,7 +2,10 @@ import { reduceAST } from './AST'
 import { type ASTNode, type EvaluatedNode, type NodeKind } from './AST/types'
 import { evaluationFunctions } from './evaluationFunctions'
 import parse from './parse'
-import parsePublicodes, { disambiguateReference } from './parsePublicodes'
+import parsePublicodes, {
+	disambiguateReference,
+	InferedUnit,
+} from './parsePublicodes'
 import {
 	getReplacements,
 	inlineReplacements,
@@ -14,7 +17,6 @@ import { formatUnit, getUnitKey } from './units'
 
 const emptyCache = (): Cache => ({
 	_meta: {
-		parentRuleStack: [],
 		evaluationRuleStack: [],
 		disableApplicabilityContextCounter: 0,
 	},
@@ -24,7 +26,6 @@ const emptyCache = (): Cache => ({
 
 type Cache = {
 	_meta: {
-		parentRuleStack: Array<string>
 		evaluationRuleStack: Array<string>
 		disableApplicabilityContextCounter: number
 		inversionFail?:
@@ -102,13 +103,19 @@ export default class Engine<Name extends string = string> {
 	// https://github.com/betagouv/publicodes/discussions/92
 	subEngines: Array<Engine<Name>> = []
 	subEngineId: number | undefined
+	ruleUnits: Record<Name, InferedUnit>
 
 	constructor(
 		rules: string | Record<string, Rule> = {},
 		options: Partial<Options> = {}
 	) {
 		this.options = { ...options, logger: options.logger ?? console }
-		this.parsedRules = parsePublicodes(rules, this.options) as ParsedRules<Name>
+		const { parsedRules, ruleUnits } = parsePublicodes<Name>(
+			rules,
+			this.options
+		)
+		this.parsedRules = parsedRules
+		this.ruleUnits = ruleUnits
 		this.replacements = getReplacements(this.parsedRules)
 	}
 
@@ -173,17 +180,9 @@ export default class Engine<Name extends string = string> {
 	evaluate(value: PublicodesExpression): EvaluatedNode
 	evaluate(value: PublicodesExpression | ASTNode): EvaluatedNode {
 		const cachedNode = this.cache.nodes.get(value)
-		// The evaluation of parent applicabilty is slightly different from
-		// regular rules since we cut some of the paths (sums) for optimization.
-		// That's why we need to have a separate cache for this evaluation.
 
 		if (cachedNode !== undefined) {
 			return cachedNode
-		} else if (this.inApplicabilityEvaluationContext) {
-			const cachedNodeApplicability = this.cache.nodesApplicability.get(value)
-			if (cachedNodeApplicability) {
-				return cachedNodeApplicability
-			}
 		}
 
 		let parsedNode: ASTNode
@@ -206,15 +205,8 @@ export default class Engine<Name extends string = string> {
 			parsedNode
 		)
 
-		// TODO: In most cases the two evaluation provide the same result, this
-		// could be optimized. The idea would be to use the “nodesApplicability”
-		// cache iff the rule uses a sum mechanism (ie, some paths are cut from
-		// the full evaluaiton).
-		if (!this.inApplicabilityEvaluationContext) {
-			this.cache.nodes.set(value, evaluatedNode)
-		} else {
-			this.cache.nodesApplicability.set(value, evaluatedNode)
-		}
+		this.cache.nodes.set(value, evaluatedNode)
+
 		return evaluatedNode
 	}
 
@@ -232,13 +224,6 @@ export default class Engine<Name extends string = string> {
 		this.subEngines.push(newEngine)
 		return newEngine
 	}
-
-	get inApplicabilityEvaluationContext(): boolean {
-		return (
-			this.cache._meta.parentRuleStack.length > 0 &&
-			this.cache._meta.disableApplicabilityContextCounter === 0
-		)
-	}
 }
 
 /**
@@ -250,44 +235,8 @@ export function UNSAFE_isNotApplicable<DottedName extends string = string>(
 	engine: Engine<DottedName>,
 	dottedName: DottedName
 ): boolean {
-	const rule = engine.getRule(dottedName)
-	return reduceAST<boolean>(
-		function (isNotApplicable, node, fn) {
-			if (isNotApplicable) return isNotApplicable
-			if (!('nodeValue' in node)) {
-				return isNotApplicable
-			}
-			if (node.nodeKind === 'variations') {
-				return node.explanation.some(
-					({ consequence }) =>
-						fn(consequence) ||
-						((consequence as any).nodeValue === null &&
-							(consequence as any).dottedName !== dottedName)
-				)
-			}
-			if (node.nodeKind === 'reference' && node.dottedName === dottedName) {
-				return fn(engine.evaluate(rule))
-			}
-			if (node.nodeKind === 'applicable si') {
-				return (
-					(node.explanation.condition as any).nodeValue === null ||
-					fn(node.explanation.valeur)
-				)
-			}
-			if (node.nodeKind === 'non applicable si') {
-				return (
-					(node.explanation.condition as any).nodeValue !== false &&
-					(node.explanation.condition as any).nodeValue !== undefined
-				)
-			}
-			if (node.nodeKind === 'rule') {
-				return (
-					(node.explanation.parent as any).nodeValue === null ||
-					fn(node.explanation.valeur)
-				)
-			}
-		},
-		false,
-		engine.evaluate(dottedName)
+	return (
+		engine.ruleUnits[dottedName].isNullable &&
+		engine.evaluate(dottedName).nodeValue === null
 	)
 }

packages/core/source/mecanisms/operation.ts

@@ -41,24 +41,10 @@ const parseOperation = (k, symbol) => (v, context) => {
 }
 
 const evaluate: EvaluationFunction<'operation'> = function (node) {
-	// When we only need to evaluate the applicability of a rule, we don't enter
-	// inside “sum terms” since we know that the sum will always be applicable.
-	// However, if somewhere in the evaluation stack we do a comparison, we need
-	// to disable this optimization since in this case we'll need the exact value
-	// of sums in the evaluation subtree.
-	const disableApplicabilityContext = ['≠', '=', '<', '>', '≤', '≥'].includes(
-		node.operator
-	)
-	if (disableApplicabilityContext && this.inApplicabilityEvaluationContext) {
-		this.cache._meta.disableApplicabilityContextCounter += 1
-	}
 	const explanation = node.explanation.map((node) => this.evaluate(node)) as [
 		EvaluatedNode,
 		EvaluatedNode
 	]
-	if (disableApplicabilityContext && this.inApplicabilityEvaluationContext) {
-		this.cache._meta.disableApplicabilityContextCounter -= 1
-	}
 	let [node1, node2] = explanation
 	const missingVariables = mergeAllMissing([node1, node2])
 

packages/core/source/mecanisms/sum.tsx

@@ -18,17 +18,4 @@ export const mecanismSum = (v, context) => {
 	} as SommeNode
 }
 
-registerEvaluationFunction('somme', function (node) {
-	if (this.inApplicabilityEvaluationContext) {
-		return {
-			// With a clearer distinction between `getApplicability` and
-			// `getValue` we could avoid faking a `nodeValue: true` and instead
-			// simply return `isApplicable: true, nodeValue: undefined`
-			nodeValue: true,
-			nodeKind: 'somme',
-			missingVariables: {},
-			explanation: [],
-		}
-	}
-	return evaluate.call(this, node)
-})
+registerEvaluationFunction('somme', evaluate)

packages/core/source/parsePublicodes.ts

@@ -15,13 +15,20 @@ export type Context = {
 	logger: Logger
 }
 
+// TODO: Currently only handle nullability, but the infering logic should be
+// extended to support the full unit type system.
+export type InferedUnit = { isNullable: boolean }
+
 type RawRule = Omit<Rule, 'nom'> | string | number
 export type RawPublicodes = Record<string, RawRule>
 
-export default function parsePublicodes(
+export default function parsePublicodes<RuleNames extends string>(
 	rawRules: RawPublicodes | string,
 	partialContext: Partial<Context> = {}
-): ParsedRules<string> {
+): {
+	parsedRules: ParsedRules<RuleNames>
+	ruleUnits: Record<RuleNames, InferedUnit>
+} {
 	// STEP 1: parse Yaml
 	let rules =
 		typeof rawRules === 'string'
@@ -61,24 +68,24 @@ export default function parsePublicodes(
 		parsedRules
 	)
 
-	// topological sort rules
-	// Throws an error if there is a cycle in the graph
-	const topologicalOrder = topologicalSort(
-		Object.keys(parsedRules),
-		dependencies
-	)
-
 	// STEP 5: Inline replacements
 	const replacements = getReplacements(parsedRules)
 	parsedRules = traverseParsedRules(
 		inlineReplacements(replacements, context.logger),
 		parsedRules
 	)
 
+	// topological sort rules
+	// Throws an error if there is a cycle in the graph
+	const topologicalOrder = topologicalSort(
+		Object.keys(parsedRules),
+		dependencies
+	)
+
 	// STEP 6: type inference
 	const ruleUnits = inferRulesUnit(parsedRules, topologicalOrder)
 
-	return parsedRules
+	return { parsedRules, ruleUnits } as any
 }
 
 // We recursively traverse the YAML tree in order to transform named parameters
@@ -178,10 +185,6 @@ function topologicalSort<Names extends string>(
 	return result
 }
 
-// TODO: Currently only handle nullability, but the infering logic should be
-// extended to support the full unit type system.
-type InferedUnit = { isNullable: boolean }
-
 function inferRulesUnit(parsedRules, topologicalOrder) {
 	const res = {}
 	topologicalOrder.forEach((ruleName) => {