diff --git a/README.md b/README.md index 151bdc4..3948218 100644 --- a/README.md +++ b/README.md @@ -67,21 +67,23 @@ flowchart TD **Prerequisites:** Java 21+, Maven 3.9+, Docker (with the Compose plugin). Node 20+ only if you want to run the web UI. -The repo ships a `docker-compose.yml` (Neo4j 5 + the `ac-code-server` container) and a `manage-ac.sh` wrapper. +The repo ships a `docker-compose.yml` (Neo4j 5 + `ac-code-server` + `ac-ui`) and a `manage-ac.sh` wrapper. ```bash git clone https://github.com/your-org/agenticcode.git cd agenticcode -# Build all modules, start Neo4j + ac-code-server, and install the `ac` CLI launcher +# Build all modules, start Neo4j + ac-code-server + ac-ui, and install the `ac` CLI launcher ./manage-ac.sh deploy ``` `./manage-ac.sh deploy` builds the project, brings the Compose stack up (leaving an already-running Neo4j untouched), -and installs the `ac` launcher to `~/.local/bin/ac`. +serves the web UI at `http://localhost:5174`, and installs the `ac` launcher to `~/.local/bin/ac`. See +[`manage-ac.sh`](#manage-acsh--the-stack-manager) below for all commands. Once up: +- **Web UI** — `http://localhost:5174` - **REST API** — `http://localhost:8787/api` - **MCP endpoint** — `http://localhost:8787/mcp/sse` (HTTP/SSE transport) - **OpenAPI / health** — `http://localhost:8787/q/openapi`, `http://localhost:8787/q/health` @@ -110,17 +112,52 @@ Re-run `ac refresh` after the sources change; it reconciles per file (unchanged `ac refresh -p upms` deep-ingests one module plus its transitive `CALLNAT`/`PERFORM` dependency tree (lazy Tier-2). -### `manage-ac.sh` subcommands +### `manage-ac.sh` — the stack manager -| Command | Action | -|-------------|----------------------------------------------------------| -| `deploy` | Build, (re)start the stack, install/refresh the `ac` CLI | -| `restart` | Recreate the `ac-code-server` container | -| `stop` | Stop the `ac-code-server` container (Neo4j left running) | -| `down` | Stop and remove the whole Compose stack | -| `status` | Show container status | -| `cli` | Rebuild + reinstall only the `ac` CLI | -| `logs [-f]` | Tail server logs (`-f` to follow) | +`manage-ac.sh` builds and runs the whole stack (Neo4j + `ac-code-server` + `ac-ui`) via docker-compose and installs the +`ac` CLI. Run it with no argument (or `help`) to print the command list — a bare invocation deliberately does **not** +deploy, since a full deploy bumps the version and rebuilds everything. + +```bash +./manage-ac.sh +``` + +| Command | What it does | +|-------------|---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------| +| `deploy` | Full deploy: bump version, `mvn clean install`, rebuild + restart `ac-code-server`, rebuild + start `ac-ui`, install/refresh the `ac` CLI. Neo4j is left running if already up. | +| `restart` | Restart `ac-code-server` only — **no build**. Also the way to abort a long server-side job (a deep refresh keeps running after its HTTP client is killed). | +| `stop` | Stop `ac-code-server` only; Neo4j and `ac-ui` keep running. | +| `down` | Stop the whole stack, Neo4j included. **The graph volume is kept** (never `down -v`). | +| `status` | Show containers, the answering server version, and the ingested projects — works even when the stack is down. | +| `cli` | Build and (re)install `ac` only — no Docker involved. | +| `ui` | Rebuild + restart `ac-ui` only (npm build runs inside Docker; no local Node needed). | +| `logs [-f]` | Last 200 lines of server logs; `-f` to follow. | + +The server answers on `http://localhost:8787`, and the **Dockerized UI on `http://localhost:5174`** (distinct from the +local Vite dev server on 5173). The version bump lives in the Maven build, so every `deploy` (a full `install`) bumps +`agenticcode.version` and re-stamps the CLI; `mvn test`/`compile`/`quarkus:dev` do not. + +### `rebuild-and-refresh.sh` — redeploy then deep-refresh + +A one-shot convenience script that redeploys the server and re-ingests the given project(s) from scratch — use it after +code changes that affect parsing or enrichment, so the graph reflects the new build. **One or more project names are +required** (there is no default; running it with no argument prints usage and exits). + +```bash +./rebuild-and-refresh.sh upms # one project +./rebuild-and-refresh.sh upms pur # several, refreshed in order +``` + +It runs the full sequence, blocking until done: **stop** the server → **`manage-ac.sh deploy`** (version bump + +server/UI rebuild) → **wait** for `http://localhost:8787/api/projects` to answer (timeout `READY_TIMEOUT`, default 300 +s) → **deep-refresh** each named project synchronously → print per-project timings and ring the terminal bell (and +`notify-send` if available). + +Overridable via env: `AC` (CLI launcher, default `ac`), `AC_SERVER_URL` (default `http://localhost:8787`), +`READY_TIMEOUT`. + +> A deep refresh is long and mutates the graph — **don't interrupt it once running**; the earlier enrichment steps are +> already committed, so an aborted refresh leaves the graph half-updated. ### Dev mode (hot reload) @@ -228,6 +265,9 @@ legacy Natural/Java and planning migrations. ### Run it +If you ran `./manage-ac.sh deploy` (or `./manage-ac.sh ui`), the UI is **already built and served in Docker +at `http://localhost:5174`** — no local Node needed. For front-end development, run the Vite dev server instead: + ```bash cd ac-ui npm install diff --git a/ac-cli/src/main/resources/agenticcode.properties b/ac-cli/src/main/resources/agenticcode.properties index 84790c4..3aa8161 100644 --- a/ac-cli/src/main/resources/agenticcode.properties +++ b/ac-cli/src/main/resources/agenticcode.properties @@ -4,4 +4,4 @@ server.url=http://localhost:8787 # Stamped by manage-ac.sh (stamp_cli_version) from ac-code-server's agenticcode.version # at build time. "dev" means this jar wasn't built via manage-ac.sh. -version=113 +version=119 diff --git a/ac-code-server/src/main/resources/application.properties b/ac-code-server/src/main/resources/application.properties index e4fa1a5..d4098f8 100644 --- a/ac-code-server/src/main/resources/application.properties +++ b/ac-code-server/src/main/resources/application.properties @@ -3,7 +3,7 @@ quarkus.http.port=8787 # AgenticCode's own release counter (not the Maven project version) — bump this by hand for each # release. Single source of truth for the startup log line, GET /api/version, and the MCP # 'version' tool/server-info (referenced below via property expression, not duplicated). -agenticcode.version=113 +agenticcode.version=119 # MCP server (HTTP/SSE transport) — tools exposed at http://:8787/mcp/sse quarkus.mcp.server.server-info.name=agenticcode diff --git a/ac-code-server/src/test/java/com/agenticcode/codeserver/api/DynamicCallOverrideIT.java b/ac-code-server/src/test/java/com/agenticcode/codeserver/api/DynamicCallOverrideIT.java index ef2f76c..de84285 100644 --- a/ac-code-server/src/test/java/com/agenticcode/codeserver/api/DynamicCallOverrideIT.java +++ b/ac-code-server/src/test/java/com/agenticcode/codeserver/api/DynamicCallOverrideIT.java @@ -189,6 +189,34 @@ class DynamicCallOverrideIT { .body("variable", hasItem("#TGT")); } + @Test + void callTreeHonoursTheOverrideLikeCallees() { + // Audit defect C: the call-tree BFS did not filter manualHidden, so it walked the marker edge the + // override only hides and reported the variable #TGT as a MODULE in the closure — while callees, + // which does filter, correctly did not. The two views must agree at every step. + resetAll(); + given().pathParam("name", "CALLERDYN") + .when().get("/api/projects/" + PROJECT + "/modules/{name}/call-tree?depth=3") + .then().statusCode(200) + .body("items.name", hasItem("#TGT")); + + Object[] site = unresolvedSite(); + setOverride((String) site[0], (Integer) site[1], List.of("TARGETMOD")); + + given().pathParam("name", "CALLERDYN") + .when().get("/api/projects/" + PROJECT + "/modules/{name}/call-tree?depth=3") + .then().statusCode(200) + .body("items.name", hasItem("TARGETMOD")) + .body("items.name", not(hasItem("#TGT"))); + + resetAll(); + given().pathParam("name", "CALLERDYN") + .when().get("/api/projects/" + PROJECT + "/modules/{name}/call-tree?depth=3") + .then().statusCode(200) + .body("items.name", hasItem("#TGT")) + .body("items.name", not(hasItem("TARGETMOD"))); + } + @Test void overrideSurvivesDeepRefresh() { resetAll(); diff --git a/ac-code-server/src/test/java/com/agenticcode/codeserver/api/NaturalViewAliasDbAccessIT.java b/ac-code-server/src/test/java/com/agenticcode/codeserver/api/NaturalViewAliasDbAccessIT.java new file mode 100644 index 0000000..1305ecd --- /dev/null +++ b/ac-code-server/src/test/java/com/agenticcode/codeserver/api/NaturalViewAliasDbAccessIT.java @@ -0,0 +1,140 @@ +package com.agenticcode.codeserver.api; + +import io.quarkus.test.junit.QuarkusTest; +import io.restassured.RestAssured; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.io.TempDir; + +import java.io.IOException; +import java.io.UncheckedIOException; +import java.nio.file.Files; +import java.nio.file.Path; + +import static io.restassured.RestAssured.given; +import static org.hamcrest.Matchers.*; + +/** + * Audit defects A and B, end to end over {@code db-accesses}. + * + *

{@code ACCESSMN0} is shaped like the {@code Y****MN0} access layer of {@code upms}: two view + * variables over one table (the generator's boilerplate {@code NEXT-VIEW} plus a + * {@code VDB2-}-prefixed one), a {@code STORE}, a labelled {@code FIND} loop, and the + * {@code UPDATE(

Before the fix this module reported three "tables" — {@code NEXT-VIEW}, + * {@code VDB2-VERSVW_THING} and the real {@code VERSVW_THING} — and no write at all for the update + * and delete, so the CRUD layer looked read-only apart from a single insert. {@code SECONDMN0} + * pins the cross-module half of the defect: it declares the same boilerplate {@code NEXT-VIEW} + * over a different table, and since {@code DB_TABLE} nodes merge on the name, the alias + * conflated the two modules onto one node. + */ +@QuarkusTest +class NaturalViewAliasDbAccessIT { + + private static final String PROJECT = "nat-view-alias"; + + private static final String ACCESSMN0 = """ + DEFINE DATA + LOCAL + 01 NEXT-VIEW VIEW OF VERSVW_THING + 02 THING_ID (N10) + 01 VDB2-VERSVW_THING VIEW OF VERSVW_THING + 02 THING_ID (N10) + 01 #ID (N10) + END-DEFINE + * + DEFINE SUBROUTINE ADD-OBJECT + STORE VDB2-VERSVW_THING + END-SUBROUTINE + * + DEFINE SUBROUTINE CHECK-EXISTENCE + EXISTENCE-CHECK. + FIND NUMBER NEXT-VIEW + WITH THING_ID = #ID + END-SUBROUTINE + * + DEFINE SUBROUTINE HOLD-OBJECT + HOLD-PRIME. + FIND VDB2-VERSVW_THING WITH + THING_ID = #ID + UPDATE(HOLD-PRIME.) + DELETE(HOLD-PRIME.) + END-FIND + END-SUBROUTINE + * + END + """; + + /** + * Same boilerplate alias name, different table — must not collapse onto one node. + */ + private static final String SECONDMN0 = """ + DEFINE DATA + LOCAL + 01 NEXT-VIEW VIEW OF VERSVW_OTHER + 02 OTHER_ID (N10) + 01 #ID (N10) + END-DEFINE + * + DEFINE SUBROUTINE CHECK-EXISTENCE + FIND NUMBER NEXT-VIEW + WITH OTHER_ID = #ID + END-SUBROUTINE + * + END + """; + + @TempDir + static Path root; + + @BeforeAll + static void ingest() { + RestAssured.port = Integer.getInteger("quarkus.http.test-port", 8081); + write("ACCESSMN0.nat", ACCESSMN0); + write("SECONDMN0.nat", SECONDMN0); + + given().contentType("application/json") + .body(new ProjectResource.ProjectRequest(null, root.toString(), null, "natural", null, null)) + .when().post("/api/projects/" + PROJECT) + .then().statusCode(201); + given().when().post("/api/projects/" + PROJECT + "/refresh?deep=true") + .then().statusCode(200); + } + + private static void write(String fileName, String content) { + try { + Files.writeString(root.resolve(fileName), content); + } catch (IOException e) { + throw new UncheckedIOException(e); + } + } + + @Test + void viewAliasesNeverSurfaceAsTables() { + given().pathParam("name", "ACCESSMN0") + .when().get("/api/projects/" + PROJECT + "/modules/{name}/db-accesses") + .then().statusCode(200) + .body("name", everyItem(equalTo("VERSVW_THING"))) + .body("name", not(hasItem("NEXT-VIEW"))) + .body("name", not(hasItem("VDB2-VERSVW_THING"))); + } + + @Test + void byReferenceUpdateAndDeleteAreRecordedAsWrites() { + // STORE (insert) plus the UPDATE/DELETE pair: three writes, not one. + given().pathParam("name", "ACCESSMN0") + .when().get("/api/projects/" + PROJECT + "/modules/{name}/db-accesses") + .then().statusCode(200) + .body("findAll { it.mode == 'WRITES' }.lineNos.flatten()", hasSize(3)) + .body("findAll { it.mode == 'READS' }.name", hasItem("VERSVW_THING")); + } + + @Test + void theSameBoilerplateAliasInTwoModulesResolvesToTwoTables() { + given().pathParam("name", "SECONDMN0") + .when().get("/api/projects/" + PROJECT + "/modules/{name}/db-accesses") + .then().statusCode(200) + .body("name", everyItem(equalTo("VERSVW_OTHER"))); + } +} diff --git a/ac-neo4j-store/src/main/java/com/agenticcode/neo4jstore/graph/CypherQueries.java b/ac-neo4j-store/src/main/java/com/agenticcode/neo4jstore/graph/CypherQueries.java index 2c425fa..d5bdb2b 100644 --- a/ac-neo4j-store/src/main/java/com/agenticcode/neo4jstore/graph/CypherQueries.java +++ b/ac-neo4j-store/src/main/java/com/agenticcode/neo4jstore/graph/CypherQueries.java @@ -840,7 +840,8 @@ public final class CypherQueries { public static final String MODULE_HOP_OUT = """ UNWIND $names AS n MATCH (m:AstNode {type: 'MODULE', name: n, project: $project}) - MATCH (m)-[:CONTAINS*0..1]->(src:AstNode)-[:CALLS]->(callee:AstNode {type: 'MODULE'}) + MATCH (m)-[:CONTAINS*0..1]->(src:AstNode)-[r:CALLS]->(callee:AstNode {type: 'MODULE'}) + WHERE coalesce(r.manualHidden, false) = false RETURN DISTINCT callee.name AS name """; @@ -851,11 +852,20 @@ public final class CypherQueries { * asked for: with {@code followWiring=true} but a {@code CALLS}-only module set, a class reached only * by injection is not in the set, the per-hop predicate prunes it, and {@code call-tree} returns an * empty list. + * + *

Both hop queries filter {@code manualHidden} exactly as {@link #callees}/{@link #callers} do. + * A manual dynamic-call override does not delete the marker edge to the variable-named placeholder + * (see {@link #DELETE_DYNAMIC_CALLNAT_PLACEHOLDER_EDGES}) — it hides it. Without the filter the BFS + * walked the hidden marker and pulled the placeholder into the closure as a {@code MODULE}, so + * {@code call-tree} listed a variable (e.g. {@code #GETSHORT-MODUL}) as a module although + * {@code callees} correctly did not, and everything driven by the BFS — {@code graph}, + * {@code db-accesses?depth=N}, {@code sql-statements?depth=N} — inherited it. */ public static final String MODULE_HOP_OUT_WIRING = """ UNWIND $names AS n MATCH (m:AstNode {type: 'MODULE', name: n, project: $project}) - MATCH (m)-[:CONTAINS*0..1]->(src:AstNode)-[:CALLS|INJECTS|REFERENCES]->(callee:AstNode {type: 'MODULE'}) + MATCH (m)-[:CONTAINS*0..1]->(src:AstNode)-[r:CALLS|INJECTS|REFERENCES]->(callee:AstNode {type: 'MODULE'}) + WHERE coalesce(r.manualHidden, false) = false RETURN DISTINCT callee.name AS name """; diff --git a/ac-parser-natural/src/main/java/com/agenticcode/parsernatural/NaturalCoarseScanner.java b/ac-parser-natural/src/main/java/com/agenticcode/parsernatural/NaturalCoarseScanner.java index 00ac68b..55bef54 100644 --- a/ac-parser-natural/src/main/java/com/agenticcode/parsernatural/NaturalCoarseScanner.java +++ b/ac-parser-natural/src/main/java/com/agenticcode/parsernatural/NaturalCoarseScanner.java @@ -63,7 +63,9 @@ public final class NaturalCoarseScanner implements CoarseScanner { private static final Pattern MACRO_ARG = Pattern.compile("'[^']*'|[#A-Za-z][#A-Za-z0-9.\\-]*"); private static AstNode dbTable(Map tables, List nodes, String name, int lineNo) { - return tables.computeIfAbsent(name, n -> { + // Upper-cased for the same reason as NaturalParser.dbTable: Natural is case-insensitive and + // DB_TABLE nodes merge on the name. + return tables.computeIfAbsent(name.toUpperCase(Locale.ROOT), n -> { AstNode table = placeholder(NodeType.DB_TABLE, n, lineNo); nodes.add(table); return table; @@ -255,6 +257,11 @@ public final class NaturalCoarseScanner implements CoarseScanner { } } + // Audit defects A/B: shared with NaturalParser rather than mirrored, so the coarse and deep + // passes cannot report different table names for the same statement — they merge on the node + // name, so a divergence would leave both a real and an alias-named DB_TABLE in the graph. + Map viewAliases = NaturalParser.viewAliases(lines); + Map loopTablesByLabel = NaturalParser.loopTablesByLabel(lines, viewAliases); Map tables = new HashMap<>(); Map workfiles = new HashMap<>(); // The enclosing subroutine of the current line (null at main-program level), so calls/DB access @@ -394,14 +401,16 @@ public final class NaturalCoarseScanner implements CoarseScanner { Matcher dbRead = DB_READ.matcher(line); if (dbRead.find()) { - AstNode table = dbTable(tables, nodes, dbRead.group(2), lineNo); + AstNode table = dbTable(tables, nodes, + NaturalParser.resolveViewAlias(viewAliases, dbRead.group(2)), lineNo); edges.add(edge(EdgeType.READS, caller, table.id(), lineNo)); continue; } Matcher dbWrite = DB_WRITE.matcher(line); if (dbWrite.find()) { - AstNode table = dbTable(tables, nodes, dbWrite.group(2), lineNo); + AstNode table = dbTable(tables, nodes, + NaturalParser.resolveViewAlias(viewAliases, dbWrite.group(2)), lineNo); edges.add(edge(EdgeType.WRITES, caller, table.id(), lineNo)); continue; } @@ -410,6 +419,18 @@ public final class NaturalCoarseScanner implements CoarseScanner { if (dbDeleteFrom.find()) { AstNode table = dbTable(tables, nodes, dbDeleteFrom.group(1), lineNo); edges.add(edge(EdgeType.WRITES, caller, table.id(), lineNo)); + continue; + } + + // Matched after DB_DELETE_FROM so the SQL form keeps precedence; an unresolvable reference + // records nothing, exactly as in NaturalParser. + Matcher dbWriteByRef = NaturalParser.DB_WRITE_BY_REF.matcher(line); + if (dbWriteByRef.find()) { + String loopTable = loopTablesByLabel.get(dbWriteByRef.group(2).toUpperCase(Locale.ROOT)); + if (loopTable != null) { + AstNode table = dbTable(tables, nodes, loopTable, lineNo); + edges.add(edge(EdgeType.WRITES, caller, table.id(), lineNo)); + } } } // Remap the copycode-expanded statement nodes/edges back to real file positions, then prepend diff --git a/ac-parser-natural/src/main/java/com/agenticcode/parsernatural/NaturalParser.java b/ac-parser-natural/src/main/java/com/agenticcode/parsernatural/NaturalParser.java index a74d182..0309884 100644 --- a/ac-parser-natural/src/main/java/com/agenticcode/parsernatural/NaturalParser.java +++ b/ac-parser-natural/src/main/java/com/agenticcode/parsernatural/NaturalParser.java @@ -69,6 +69,21 @@ public final class NaturalParser implements LanguageParser { private static final Pattern END_FIND = Pattern.compile("(?i)^\\s*END-FIND\\b"); private static final Pattern END_READ = Pattern.compile("(?i)^\\s*END-READ\\b"); private static final Pattern VIEW_OF = Pattern.compile("(?i)\\bVIEW\\s+OF\\s+(\\S+)"); + // Natural DML by reference: `UPDATE (r)` / `DELETE (r)` act on the current record of the loop + // identified by r — a statement label (`HOLD-PRIME.`) or, in a form this corpus does not use, a + // source-line number. The operand is never a view, so the table comes from the referenced loop + // (audit defect B). Whitespace before `(` is optional: the corpus writes `UPDATE(HOLD-PRIME.)`. + static final Pattern DB_WRITE_BY_REF = + Pattern.compile("(?i)^\\s*(UPDATE|DELETE)\\s*\\(\\s*([A-Z0-9#@$&\\-_.]+?)\\.?\\s*\\)"); + // A `DEFINE DATA` view declaration ` VIEW OF `: the alias is a *variable*, and + // it — not the table — is what every Natural DML statement names. Resolving it is what keeps + // `db-accesses` reporting real tables (audit defect A); without it a module that only ever touches + // VERSVW_LITERALES reports three "tables", one of them the generator's boilerplate name NEXT-VIEW, + // which then collides across every access layer that uses the same boilerplate. + private static final Pattern VIEW_DECL = + Pattern.compile("(?i)^\\s*\\d+\\s+([A-Z0-9#@$&\\-_]+)\\s+VIEW\\s+OF\\s+(\\S+)"); + // A statement label introducing the FIND/READ on the following line (`HOLD-PRIME.` on its own line). + private static final Pattern STATEMENT_LABEL = Pattern.compile("^\\s*([A-Z0-9#@$&\\-_]+)\\.\\s*(?:/\\*.*)?$"); // STORE takes a real ADABAS view operand. UPDATE has two forms: `UPDATE ` (real) and Natural // DML `UPDATE (label)` (updates the current record of the enclosing loop via a reference label — no // view); DELETE only ever has the latter shape (`DELETE [(label)]`, or the EXAMINE clause @@ -176,13 +191,100 @@ public final class NaturalParser implements LanguageParser { Pattern.compile("(?i)^\\s*DEFINE\\s+SUBROUTINE\\s+(GET-XML-LINE|GET-XML-ACT)\\b"); private static AstNode dbTable(Map dbTables, List nodes, String name, int lineNo) { - return dbTables.computeIfAbsent(name, n -> { + // Natural is case-insensitive, and DB_TABLE nodes merge on (type, name, sourceFile="") — so a + // lower-case statement would otherwise mint a second node for a table already known upper-case. + return dbTables.computeIfAbsent(name.toUpperCase(Locale.ROOT), n -> { AstNode table = node(NodeType.DB_TABLE, n, "", lineNo, lineNo, null, null); nodes.add(table); return table; }); } + /** + * Audit defect A: maps each {@code DEFINE DATA} view alias to the table it is declared over + * ({@code 1 NEXT-VIEW VIEW OF VERSVW_LITERALES} → {@code NEXT-VIEW} → + * {@code VERSVW_LITERALES}), so a Natural DML operand can be resolved to a real table. + * + *

Runs over the copycode-expanded lines, so an alias declared in an included {@code .cpy} data + * block is seen too. + */ + static Map viewAliases(String[] lines) { + Map aliases = new HashMap<>(); + for (String line : lines) { + Matcher m = VIEW_DECL.matcher(line); + if (m.find()) { + aliases.putIfAbsent(m.group(1).toUpperCase(Locale.ROOT), m.group(2).toUpperCase(Locale.ROOT)); + } + } + return aliases; + } + + /** + * The table a Natural DML operand denotes: the view alias resolved, or the operand itself. + */ + static String resolveViewAlias(Map aliases, String operand) { + return aliases.getOrDefault(operand.toUpperCase(Locale.ROOT), operand); + } + + /** + * Audit defect B: maps a {@code FIND}/{@code READ} statement label to the (alias-resolved) table its + * loop reads, so {@code UPDATE(

Deliberately label-only. Natural also allows a bare {@code UPDATE}/{@code DELETE} and a + * source-line reference, both of which would need the enclosing-loop extent to resolve; neither + * occurs in this corpus, and guessing an enclosing loop is how phantom tables got in before. An + * unresolvable reference stays unrecorded. + */ + static Map loopTablesByLabel(String[] lines, Map aliases) { + Map byLabel = new HashMap<>(); + for (int i = 0; i < lines.length; i++) { + Matcher read = DB_READ.matcher(lines[i]); + if (read.find()) { + String label = precedingStatementLabel(lines, i); + if (label != null) { + byLabel.putIfAbsent(label, resolveViewAlias(aliases, read.group(2))); + } + continue; + } + // A labelled SQL `SELECT` is a loop too, and the generated access layer holds its record that + // way as often as with a FIND (YELEMMN0/YMULTMN0 do). Its table is the FROM operand — already + // a real table, so no alias resolution applies. + if (SELECT_FROM.matcher(lines[i]).find()) { + String label = precedingStatementLabel(lines, i); + if (label == null) { + continue; + } + for (int j = i; j < lines.length; j++) { + Matcher from = FROM_VIEW.matcher(lines[j]); + if (from.find()) { + byLabel.putIfAbsent(label, from.group(1).toUpperCase(Locale.ROOT)); + break; + } + if (END_SELECT.matcher(lines[j]).find()) { + break; + } + } + } + } + return byLabel; + } + + /** + * The statement label on the line before {@code idx}, skipping blank and comment lines. + */ + private static @Nullable String precedingStatementLabel(String[] lines, int idx) { + for (int j = idx - 1; j >= 0; j--) { + String candidate = lines[j].trim(); + if (candidate.isEmpty() || candidate.startsWith("*")) { + continue; + } + Matcher label = STATEMENT_LABEL.matcher(lines[j]); + return label.matches() ? label.group(1).toUpperCase(Locale.ROOT) : null; + } + return null; + } + private static AstNode workfile(Map workfiles, List nodes, String number, @Nullable String physicalName, int lineNo) { return workfiles.computeIfAbsent(number, n -> { @@ -924,6 +1026,12 @@ public final class NaturalParser implements LanguageParser { workfilePhysicalNames.putIfAbsent(wd.group(1), wd.group(2).trim()); } } + // Pre-scan the view declarations and the labelled FIND/READ loops: both resolve a DML operand to a + // real table, and both must be known before the first statement is seen (a label may be declared + // after the write that references it only in generated code, but the cost of scanning up front is + // one pass and it removes the ordering question entirely). + Map viewAliases = viewAliases(lines); + Map loopTablesByLabel = loopTablesByLabel(lines, viewAliases); Map dataStructures = new HashMap<>(); Map variables = new HashMap<>(); Map placeholderFields = new HashMap<>(); @@ -1267,7 +1375,7 @@ public final class NaturalParser implements LanguageParser { Matcher dbWriteMatcher = DB_WRITE.matcher(line); if (dbWriteMatcher.find()) { - AstNode table = dbTable(dbTables, nodes, dbWriteMatcher.group(2), lineNo); + AstNode table = dbTable(dbTables, nodes, resolveViewAlias(viewAliases, dbWriteMatcher.group(2)), lineNo); edges.add(edge(EdgeType.WRITES, caller, table.id(), lineNo)); AstNode access = node(NodeType.DB_ACCESS, table.name(), sourceFile, lineNo, lineNo, "WRITE", line.trim()); nodes.add(access); @@ -1287,10 +1395,29 @@ public final class NaturalParser implements LanguageParser { continue; } + // Matched after DB_DELETE_FROM so the SQL form `DELETE FROM

` keeps precedence. + Matcher dbWriteByRefMatcher = DB_WRITE_BY_REF.matcher(line); + if (dbWriteByRefMatcher.find()) { + String loopTable = loopTablesByLabel.get(dbWriteByRefMatcher.group(2).toUpperCase(Locale.ROOT)); + if (loopTable != null) { + String verb = dbWriteByRefMatcher.group(1).toUpperCase(Locale.ROOT); + AstNode table = dbTable(dbTables, nodes, loopTable, lineNo); + edges.add(edge(EdgeType.WRITES, caller, table.id(), lineNo)); + AstNode access = node(NodeType.DB_ACCESS, table.name(), sourceFile, lineNo, lineNo, + "DELETE".equals(verb) ? "DELETE" : "WRITE", line.trim()); + nodes.add(access); + edges.add(edge(EdgeType.CONTAINS, caller, access.id(), lineNo)); + edges.add(edge(EdgeType.USES_TYPE, access.id(), table.id(), lineNo)); + } + // An unresolvable reference (unknown label, or the numeric source-line form) records + // nothing: no enclosing loop is guessed, so no phantom table can be minted. + continue; + } + Matcher dbReadMatcher = DB_READ.matcher(line); if (dbReadMatcher.find()) { String verb = dbReadMatcher.group(1).toUpperCase(Locale.ROOT); - AstNode table = dbTable(dbTables, nodes, dbReadMatcher.group(2), lineNo); + AstNode table = dbTable(dbTables, nodes, resolveViewAlias(viewAliases, dbReadMatcher.group(2)), lineNo); edges.add(edge(EdgeType.READS, caller, table.id(), lineNo)); // P1-j: collect multi-line statement body up to END-FIND / END-READ Pattern endPattern = "FIND".equals(verb) ? END_FIND : END_READ; diff --git a/ac-parser-natural/src/test/java/com/agenticcode/parsernatural/NaturalCoarseScannerTest.java b/ac-parser-natural/src/test/java/com/agenticcode/parsernatural/NaturalCoarseScannerTest.java index 4358ec2..7543aeb 100644 --- a/ac-parser-natural/src/test/java/com/agenticcode/parsernatural/NaturalCoarseScannerTest.java +++ b/ac-parser-natural/src/test/java/com/agenticcode/parsernatural/NaturalCoarseScannerTest.java @@ -276,4 +276,34 @@ class NaturalCoarseScannerTest { assertFalse(hasNode(r, NodeType.MODULE, "USIA008N"), "a real module named in a literal is not a call"); } + + @Test + void viewAliasesAndByReferenceWritesResolveExactlyAsInTheDeepPass() { + // Audit defects A/B: the coarse pass shares NaturalParser's resolution. If it drifted, a shallow + // module would report the alias and a FULL one the table — and since DB_TABLE nodes merge on the + // name, both would end up in the graph for the same table. + String src = """ + DEFINE DATA LOCAL + 1 VDB2-T_REAL VIEW OF T_REAL + 2 REC-ID (N10) + END-DEFINE + HOLD-PRIME. + FIND VDB2-T_REAL WITH + REC-ID = 1 + UPDATE(HOLD-PRIME.) + DELETE(HOLD-PRIME.) + END-FIND + END + """; + ParseResult r = scanner.scan("PGM.nat", src); + assertTrue(hasNode(r, NodeType.DB_TABLE, "T_REAL"), "the underlying table is indexed"); + assertFalse(hasNode(r, NodeType.DB_TABLE, "VDB2-T_REAL"), "the view alias is not a table"); + assertFalse(hasNode(r, NodeType.DB_TABLE, "HOLD-PRIME"), "the statement label is not a table"); + assertEquals(2, r.edges().stream() + .filter(e -> e.type() == EdgeType.WRITES) + .filter(e -> r.nodes().stream().anyMatch(n -> n.id().equals(e.targetId()) + && n.type() == NodeType.DB_TABLE && n.name().equals("T_REAL"))) + .count(), + "UPDATE(label.) and DELETE(label.) each write the loop's table"); + } } diff --git a/ac-parser-natural/src/test/java/com/agenticcode/parsernatural/NaturalParserTest.java b/ac-parser-natural/src/test/java/com/agenticcode/parsernatural/NaturalParserTest.java index 3f12efa..2bfff4d 100644 --- a/ac-parser-natural/src/test/java/com/agenticcode/parsernatural/NaturalParserTest.java +++ b/ac-parser-natural/src/test/java/com/agenticcode/parsernatural/NaturalParserTest.java @@ -32,6 +32,28 @@ class NaturalParserTest { return result.nodes().stream().anyMatch(n -> n.type() == type && n.name().equals(name)); } + /** + * The ascending line numbers of {@code type} edges from {@code source} to the DB_TABLE {@code table}. + */ + private static List edgeLines(LanguageParser.ParseResult result, EdgeType type, AstNode source, String table) { + return result.edges().stream() + .filter(e -> e.type() == type && e.sourceId().equals(source.id())) + .filter(e -> result.nodes().stream().anyMatch( + n -> n.id().equals(e.targetId()) && n.type() == NodeType.DB_TABLE && n.name().equals(table))) + .map(AstEdge::lineNo) + .distinct() + .sorted() + .toList(); + } + + private static List readLines(LanguageParser.ParseResult result, AstNode source, String table) { + return edgeLines(result, EdgeType.READS, source, table); + } + + private static List writeLines(LanguageParser.ParseResult result, AstNode source, String table) { + return edgeLines(result, EdgeType.WRITES, source, table); + } + private static boolean hasEdge(LanguageParser.ParseResult result, EdgeType type, AstNode source, String targetName, NodeType targetType) { return result.edges().stream().anyMatch(e -> e.type() == type && e.sourceId().equals(source.id()) @@ -910,6 +932,129 @@ class NaturalParserTest { "UPDATE still records the real view"); } + @Test + void viewAliasResolvesToTheUnderlyingTable() { + // Audit defect A: a Natural DML operand is a view *variable* (`1 VIEW OF
`), not the + // table. Reporting the alias mints a phantom DB_TABLE and splits one table across several names — + // in upms a single node `NEXT-VIEW` stood for 11 different tables. + String content = """ + DEFINE DATA LOCAL + 1 NEXT-VIEW VIEW OF T_REAL + 2 REC-ID (N10) + 1 VDB2-T_REAL VIEW OF T_REAL + 2 REC-ID (N10) + END-DEFINE + DEFINE SUBROUTINE ACCESS-IT + FIND NUMBER NEXT-VIEW + WITH REC-ID = 1 + FIND VDB2-T_REAL WITH + REC-ID = 2 + END-FIND + STORE VDB2-T_REAL + END-SUBROUTINE + END + """; + + LanguageParser.ParseResult result = parser.parse("VIEW_ALIAS_SAMPLE.nat", content); + + AstNode access = findNode(result, NodeType.FUNCTION, "ACCESS-IT"); + assertFalse(hasNode(result, NodeType.DB_TABLE, "NEXT-VIEW"), + "The view variable NEXT-VIEW must not become a DB_TABLE"); + assertFalse(hasNode(result, NodeType.DB_TABLE, "VDB2-T_REAL"), + "The view variable VDB2-T_REAL must not become a DB_TABLE"); + assertTrue(hasEdge(result, EdgeType.READS, access, "T_REAL", NodeType.DB_TABLE), + "FIND through a view alias must READ the underlying table"); + assertTrue(hasEdge(result, EdgeType.WRITES, access, "T_REAL", NodeType.DB_TABLE), + "STORE through a view alias must WRITE the underlying table"); + assertEquals(List.of(8, 10), readLines(result, access, "T_REAL"), + "Both FIND variants (incl. FIND NUMBER) must resolve to T_REAL"); + } + + @Test + void updateAndDeleteByReferenceResolveToTheEnclosingLoopTable() { + // Audit defect B: `UPDATE(label.)` / `DELETE(label.)` write the current record of the labelled + // FIND/READ loop. Dropping them (to avoid a phantom `(label.)` table) made the whole Y****MN0 CRUD + // layer look read-only. The loop operand is itself a view alias, so A and B compose. + String content = """ + DEFINE DATA LOCAL + 1 VDB2-T_REAL VIEW OF T_REAL + 2 REC-ID (N10) + END-DEFINE + DEFINE SUBROUTINE HOLD-OBJECT + HOLD-PRIME. + FIND VDB2-T_REAL WITH + REC-ID = 1 + UPDATE(HOLD-PRIME.) + DELETE(HOLD-PRIME.) + END-FIND + END-SUBROUTINE + END + """; + + LanguageParser.ParseResult result = parser.parse("BY_REF_SAMPLE.nat", content); + + AstNode hold = findNode(result, NodeType.FUNCTION, "HOLD-OBJECT"); + assertFalse(hasNode(result, NodeType.DB_TABLE, "HOLD-PRIME"), + "The statement label must not become a DB_TABLE"); + assertEquals(List.of(9, 10), writeLines(result, hold, "T_REAL"), + "UPDATE(label.) and DELETE(label.) must WRITE the loop's table"); + } + + @Test + void byReferenceWriteResolvesThroughALabelledSelectLoop() { + // Audit defect B, second shape: the generated access layer holds its record with a labelled SQL + // SELECT as often as with a FIND (YELEMMN0, YMULTMN0). The table is the FROM operand. + String content = """ + DEFINE DATA LOCAL + 1 VDB2-T_REAL VIEW OF T_REAL + 2 REC-ID (N10) + END-DEFINE + DEFINE SUBROUTINE HOLD-OBJECT + HOLD-PRIME. + SELECT * + INTO VIEW VDB2-T_REAL + FROM T_REAL + WHERE REC-ID = 1 + UPDATE(HOLD-PRIME.) + DELETE(HOLD-PRIME.) + END-SELECT + END-SUBROUTINE + END + """; + + LanguageParser.ParseResult result = parser.parse("SELECT_LABEL_SAMPLE.nat", content); + + AstNode hold = findNode(result, NodeType.FUNCTION, "HOLD-OBJECT"); + assertEquals(List.of(11, 12), writeLines(result, hold, "T_REAL"), + "A by-reference write must resolve through a labelled SELECT loop too"); + } + + @Test + void byReferenceWriteWithoutAResolvableLoopNamesNoTable() { + // The no-phantom guarantee must survive the defect-B fix: an unmatched label, and the numeric + // source-line form `UPDATE (r)` that Natural also allows, resolve to nothing rather than to a + // guessed table. + String content = """ + DEFINE DATA LOCAL + 1 VDB2-T_REAL VIEW OF T_REAL + 2 REC-ID (N10) + END-DEFINE + DEFINE SUBROUTINE SAVE + UPDATE(NO-SUCH-LABEL.) + DELETE(0100) + END-SUBROUTINE + END + """; + + LanguageParser.ParseResult result = parser.parse("BY_REF_UNRESOLVED_SAMPLE.nat", content); + + AstNode save = findNode(result, NodeType.FUNCTION, "SAVE"); + assertFalse(hasNode(result, NodeType.DB_TABLE, "NO-SUCH-LABEL"), "Unmatched label names no table"); + assertFalse(hasNode(result, NodeType.DB_TABLE, "0100"), "A source-line reference names no table"); + assertTrue(writeLines(result, save, "T_REAL").isEmpty(), + "An unresolvable by-reference write must not be attributed to any table"); + } + @Test void multiLineFindStatementTextIsCapturedFully() { // P1-j: FIND spanning multiple lines must collect all lines until END-FIND diff --git a/rebuild-and-refresh.sh b/rebuild-and-refresh.sh index 852a0a5..8734c97 100755 --- a/rebuild-and-refresh.sh +++ b/rebuild-and-refresh.sh @@ -1,12 +1,17 @@ #!/usr/bin/env bash # # rebuild-and-refresh.sh — stop the server, rebuild + redeploy it, then run a deep refresh -# of both projects (upms, pur) and announce when both refreshes have finished. +# of the given project(s) and announce when all refreshes have finished. # # Usage: -# ./rebuild-and-refresh.sh +# ./rebuild-and-refresh.sh [ ...] +# +# Example: +# ./rebuild-and-refresh.sh upms +# ./rebuild-and-refresh.sh upms pur # # Notes: +# * At least one project is required (no default) — the script exits with usage if none is given. # * A deep refresh is long and mutates the graph — do not interrupt it once running. # * `manage-ac.sh deploy` already bumps the version, rebuilds ac-code-server + ac-ui and # brings the stack up; we stop first (explicit) so the sequence is unambiguous. @@ -20,10 +25,17 @@ AC="${AC:-ac}" # ac-cli launcher (on PATH: ~/.local SERVER_URL="${AC_SERVER_URL:-http://localhost:8787}" READY_PROBE="$SERVER_URL/api/projects" READY_TIMEOUT="${READY_TIMEOUT:-300}" # seconds to wait for the server to come up -PROJECTS=(upms pur) -log() { printf '\n\033[1;34m[%(%H:%M:%S)T] %s\033[0m\n' -1 "$*"; } -fail() { printf '\n\033[1;31m[%(%H:%M:%S)T] %s\033[0m\n' -1 "$*" >&2; exit 1; } +log() { printf '\n\033[1;34m[%(%H:%M:%S)T] %s\033[0m\n' -1 "$*"; } +fail() { printf '\n\033[1;31m[%(%H:%M:%S)T] %s\033[0m\n' -1 "$*" >&2; exit 1; } +usage() { echo "Usage: ./rebuild-and-refresh.sh [ ...]" >&2; exit 2; } + +# Mandatory: one or more projects to deep-refresh, given as arguments. +if (( $# == 0 )); then + echo "Error: no project given — at least one is required." >&2 + usage +fi +PROJECTS=("$@") # 1. Stop the server ----------------------------------------------------------------------- log "Stopping ac-code-server ..." diff --git a/x-docs/mcp-api-usage-ac-implementation.md b/x-docs/mcp-api-usage-ac-implementation.md index 5e03ec6..7de3fa2 100644 --- a/x-docs/mcp-api-usage-ac-implementation.md +++ b/x-docs/mcp-api-usage-ac-implementation.md @@ -283,6 +283,34 @@ module. Before item 93 the transitive query carried only the `READS`/`WRITES` br *same* module with `depth` dropped its declared table and a Java caller's transitive `db-accesses` came back empty although the entity it persists through maps to a real table. +**Natural view aliases are resolved to the underlying table (item 95).** A Natural DML statement names a +*view variable* (`1 VDB2-VERSIS_LITERALES VIEW OF VERSVW_LITERALES`), not the DDM. `db-accesses` reports +the **table** — `FIND VDB2-VERSIS_LITERALES`, `FIND NUMBER NEXT-VIEW` and `STORE VDB2-VERSIS_LITERALES` +in `YLITEMN0` all come back as `VERSVW_LITERALES`, matching the SQL `SELECT … FROM` rows in the same +module. Before item 95 the alias itself was the reported name, which (a) split one table across several +names, (b) made the generator's boilerplate alias `NEXT-VIEW` a single node shared by 11 modules meaning +11 different tables, and (c) hid every `VERSVW_LOGFILE` write behind 11 `VDB2-*-VLOG` aliases. Table +names are upper-cased (Natural is case-insensitive). + +**Natural `UPDATE(