From 203f8dc13135d13df3fbf39283a4e6159dd5fce5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9mi=20C=C3=A9r=C3=A8s?= <remi.ceres@inria.fr>
Date: Fri, 18 Sep 2026 14:51:41 +0200
Subject: [PATCH] test(architecture): enforce exact engine-to-AST boundary
 guardrail and document existing exceptions

---
 docs/next-engine-ast-boundary.md              | 62 ++++++++++++++
 .../next/query/impl/engine/package-info.java  |  8 +-
 .../architecture/NextModuleBoundaryTest.java  | 82 +++++++++++++++++--
 3 files changed, 144 insertions(+), 8 deletions(-)
 create mode 100644 docs/next-engine-ast-boundary.md

diff --git a/docs/next-engine-ast-boundary.md b/docs/next-engine-ast-boundary.md
new file mode 100644
index 000000000..e4ab1056c
--- /dev/null
+++ b/docs/next-engine-ast-boundary.md
@@ -0,0 +1,62 @@
+# Engine / AST dependency boundary
+
+The target is an execution engine independent of parser and SPARQL AST types.
+The current engine still exposes three dependencies. `NextModuleBoundaryTest`
+inventories them as exact source-path / referenced-type pairs, rather than
+exempting whole files or packages.
+
+## Existing exceptions
+
+Paths below are relative to `next/query/impl/engine`.
+
+| Source | AST type | Current use and callers |
+| --- | --- | --- |
+| `model/Filter.java` | `TermAst` | Return types of `getFilterExpression()` and `coreseNextSource()`. `NextFilterFromAst` implements both in the bridge. `Exp` delegates the first; `AstBackedExprTest` checks its result. No production caller of `coreseNextSource()` was found in `next`. |
+| `pattern/Exp.java` | `TermAst` | Return type of `getFilterExpression()`, delegating to `Filter`. No production caller of this accessor was found in `next`. |
+| `pattern/Query.java` | `QueryAst` | Field `ast` and accessors `getAST()`, `getGlobalAST()`, `setAST()`. `CoreseAstQueryBuilder` sets the source AST for each query form; `QueryTest` tests the accessors. No external production reader of these accessors was found in `next`. |
+
+This is a source-reference inventory, not a statement that these APIs can be
+removed without checking downstream clients. Bridge implementations may still
+own AST objects internally after the engine contracts stop exposing their types.
+
+## Guard behavior
+
+- Parser dependencies have no exceptions.
+- Both the actual `query.impl.sparql.ast` package and the historical
+  `query.impl.ast` package are checked.
+- Imports, wildcard imports, static references and fully qualified references
+  are detected by the existing source-reference matching approach.
+- Adding a type to an already listed file is rejected.
+- Adding an existing AST type to another file is rejected.
+- Removing a dependency requires removing its exception in the same change.
+
+The source scanner also sees fully qualified names in comments and strings. It
+does not perform bytecode or transitive dependency analysis. Its regression test
+uses synthetic sources to check new files, new types, wildcard imports and fully
+qualified references outside imports.
+
+## Follow-up: remove the exceptions
+
+1. Check repository-wide and downstream uses of the source-AST accessors before
+   removing them; assess compatibility of these implementation classes.
+2. Remove the unused `Exp` forwarding accessor and the AST accessors from the
+   engine `Filter` contract. Keep source-AST access inside the bridge where needed;
+   adapt `AstBackedExprTest` to test bridge behavior rather than an engine AST API.
+3. Remove AST retention from `Query` if no required consumer exists. If diagnostics
+   need source information, keep it in a bridge-owned structure with an explicit
+   lifecycle, rather than replacing typed accessors with `Object` or unsafe casts.
+4. Remove the corresponding builder writes and accessor-only tests. Preserve
+   behavioral tests for query forms, expressions and subqueries.
+5. Delete each exception as its dependency disappears, then restore the strict
+   no-AST-dependencies invariant in the engine package documentation.
+
+Do not introduce new runtime behavior as part of this follow-up. Validate with
+the architecture tests, the full core test suite and a comparison against the
+existing W3C baseline.
+
+## Validation
+
+```bash
+./gradlew test --tests '*NextModuleBoundaryTest'
+./gradlew check -x test
+```
diff --git a/src/main/java/fr/inria/corese/core/next/query/impl/engine/package-info.java b/src/main/java/fr/inria/corese/core/next/query/impl/engine/package-info.java
index 47bd9f318..190301ed2 100644
--- a/src/main/java/fr/inria/corese/core/next/query/impl/engine/package-info.java
+++ b/src/main/java/fr/inria/corese/core/next/query/impl/engine/package-info.java
@@ -7,9 +7,11 @@
  *
  * <h2>Architectural Invariants &amp; Development Guardrails</h2>
  * <ul>
- *   <li><b>Zero AST Leakage</b>: No direct dependencies exist from execution engine
- *   components ({@code eval}, {@code pattern}, {@code solution}, {@code memory})
- *   to ANTLR or parser AST classes. The engine executes compiled logical operators only.</li>
+ *   <li><b>AST Boundary</b>: New dependencies on parser or AST classes are forbidden.
+ *   Three existing source/type dependencies in {@code Filter}, {@code Exp}, and
+ *   {@code Query} remain explicitly inventoried by {@code NextModuleBoundaryTest}.
+ *   Removing these exceptions is tracked in {@code docs/next-engine-ast-boundary.md};
+ *   the target architecture executes compiled logical operators only.</li>
  *   <li><b>Storage Isolation</b>: Query evaluation and triple access must strictly
  *   transit through the {@link fr.inria.corese.core.next.query.impl.engine.spi.Producer}
  *   and Storage SPI abstractions.</li>
diff --git a/src/test/java/fr/inria/corese/core/next/architecture/NextModuleBoundaryTest.java b/src/test/java/fr/inria/corese/core/next/architecture/NextModuleBoundaryTest.java
index 1bdc7115d..1df6f0ec0 100644
--- a/src/test/java/fr/inria/corese/core/next/architecture/NextModuleBoundaryTest.java
+++ b/src/test/java/fr/inria/corese/core/next/architecture/NextModuleBoundaryTest.java
@@ -1,6 +1,7 @@
 package fr.inria.corese.core.next.architecture;
 
 import static org.junit.jupiter.api.Assertions.fail;
+import static org.junit.jupiter.api.Assertions.assertEquals;
 
 import java.io.IOException;
 import java.nio.file.Files;
@@ -15,6 +16,7 @@
 import java.util.stream.Stream;
 
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
 
 /** Protects the dependency direction between the top-level {@code next} modules. */
 class NextModuleBoundaryTest {
@@ -25,6 +27,13 @@ class NextModuleBoundaryTest {
             "\\bfr\\.inria\\.corese\\.core(?:\\.[A-Za-z_$][A-Za-z0-9_$]*)+");
     private static final Pattern STATIC_IMPORT_PREFIX = Pattern.compile("^static\\s+");
 
+    // Exact source/type pairs only: see docs/next-engine-ast-boundary.md.
+    // Equality below also requires removing exceptions when their dependencies disappear.
+    private static final Set<String> EXISTING_ENGINE_AST_DEPENDENCIES = Set.of(
+            "model/Filter.java -> fr.inria.corese.core.next.query.impl.sparql.ast.TermAst",
+            "pattern/Exp.java -> fr.inria.corese.core.next.query.impl.sparql.ast.TermAst",
+            "pattern/Query.java -> fr.inria.corese.core.next.query.impl.sparql.ast.QueryAst");
+
     @Test
     void sharedCodeMustNotDependOnDomainModules() throws IOException {
         assertNoImports(
@@ -88,12 +97,75 @@ void queryRuntimeMustNotDependOnTheLegacyPipeline() throws IOException {
     }
 
     @Test
-    void queryEngineMustNotDependOnParserOrAst() throws IOException {
+    void queryEngineMustNotAddParserOrAstDependencies() throws IOException {
         Path engineSources = NEXT_SOURCES.resolve("query/impl/engine");
-        assertNoReferences(
-                engineSources,
-                reference -> reference.startsWith("fr.inria.corese.core.next.query.impl.sparql.parser")
-                        || reference.startsWith("fr.inria.corese.core.next.query.impl.ast"));
+        assertEquals(EXISTING_ENGINE_AST_DEPENDENCIES, engineSyntaxDependencies(engineSources),
+                "Unexpected engine syntax dependency, or obsolete exception: "
+                        + "remove dependencies rather than expanding the exception list.");
+    }
+
+    @Test
+    void engineBoundaryFindsSyntaxReferencesBeyondExistingExceptions(@TempDir Path sources)
+            throws IOException {
+        Path filter = sources.resolve("model/Filter.java");
+        Files.createDirectories(filter.getParent());
+        Files.writeString(filter, """
+                import fr.inria.corese.core.next.query.impl.sparql.ast.TermAst;
+                import fr.inria.corese.core.next.query.impl.sparql.ast.QueryAst;
+                import fr.inria.corese.core.next.query.impl.sparql.parser.SparqlParser;
+                import fr.inria.corese.core.next.query.impl.sparql.ast.*;
+                class Filter {
+                    fr.inria.corese.core.next.query.impl.sparql.ast.VarAst variable;
+                }
+                """);
+        Files.writeString(sources.resolve("NewOperator.java"),
+                "import fr.inria.corese.core.next.query.impl.sparql.ast.TermAst;");
+        Set<String> unexpected = engineSyntaxDependencies(sources);
+        unexpected.removeAll(EXISTING_ENGINE_AST_DEPENDENCIES);
+        assertEquals(Set.of(
+                "model/Filter.java -> fr.inria.corese.core.next.query.impl.sparql.ast.QueryAst",
+                "model/Filter.java -> fr.inria.corese.core.next.query.impl.sparql.parser.SparqlParser",
+                "model/Filter.java -> fr.inria.corese.core.next.query.impl.sparql.ast.VarAst",
+                "model/Filter.java -> fr.inria.corese.core.next.query.impl.sparql.ast",
+                "NewOperator.java -> fr.inria.corese.core.next.query.impl.sparql.ast.TermAst"), unexpected);
+    }
+
+    /**
+     * Inventories syntax dependencies by source path and fully qualified reference.
+     *
+     * @param sources engine source root
+     * @return mutable set of dependency pairs, including references outside imports
+     * @throws IOException if sources cannot be read
+     */
+    private static Set<String> engineSyntaxDependencies(Path sources) throws IOException {
+        Set<String> dependencies = new LinkedHashSet<>();
+        try (Stream<Path> paths = Files.walk(sources)) {
+            for (Path source : paths.filter(path -> path.toString().endsWith(".java")).toList()) {
+                Matcher matcher = CORESE_TYPE_REFERENCE.matcher(Files.readString(source));
+                while (matcher.find()) {
+                    String reference = matcher.group();
+                    if (isQuerySyntaxReference(reference)) {
+                        dependencies.add(sources.relativize(source).toString().replace('\\', '/')
+                                + " -> " + reference);
+                    }
+                }
+            }
+        }
+        return dependencies;
+    }
+
+    /**
+     * Recognizes parser and AST packages, including the historical AST location.
+     *
+     * @param reference fully qualified source reference
+     * @return whether the reference crosses the engine/syntax boundary
+     */
+    private static boolean isQuerySyntaxReference(String reference) {
+        return List.of(
+                "fr.inria.corese.core.next.query.impl.sparql.parser",
+                "fr.inria.corese.core.next.query.impl.sparql.ast",
+                "fr.inria.corese.core.next.query.impl.ast").stream()
+                .anyMatch(prefix -> reference.equals(prefix) || reference.startsWith(prefix + "."));
     }
 
     @Test