diff --git a/CLAUDE.md b/CLAUDE.md index 209abdc..e3e16ac 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -6,7 +6,8 @@ exceptions. If you need to ask something, use `AskUserQuestion`. If you need clarification, use `AskUserQuestion`. If you need a decision, use `AskUserQuestion`. The tool provides a freetext option automatically — use it. * **Never assume anything about the user's intent.** Ask questions and wait for the user to clarify. -* **Always use agentic code** see x-docs/mcp-api-usage-ac-implementation.md to understand the code, get an overview and +* **Always use agentic code** see x-docs/agent-api-usage-ac-implementation.md to understand the code, get an overview + and if somethinmg is missing or not working, report it directly. Also. find identifier via agentic code. if agentic code is not running, report it. * **NEVER start Docker yourself — the human starts it.** If you need the server/Neo4j for analysis and @@ -45,7 +46,8 @@ - All features must be tracked in `x-docs/roadmap.md`. Once a feature has been implemented, mark it `[x]` and add a timestamp (date) indicating when it was completed. - For **every feature** that changes API behavior or what an agent can query, you MUST update - `x-docs/mcp-api-usage-ac-implementation.md` to reflect it (new/changed endpoints, response fields, semantics) — treat + `x-docs/agent-api-usage-ac-implementation.md` to reflect it (new/changed endpoints, response fields, semantics) — + treat this doc update as part of the feature's Definition of Done, not an optional follow-up. - Never add a `Co-Authored-By:` line (or any AI-attribution trailer) to commit messages. @@ -53,7 +55,7 @@ server runs at `http://localhost:8787` with this repo already ingested as project `ac`. Prefer it over grep/Explore for call graphs, callers/callees, DB access, dataflow, and module overviews — it's exactly the tool this project builds, so using it here is both faster and the best test of its own - output. Usage guide: `x-docs/mcp-api-usage-ac-implementation.md`. Re-ingest after code changes + output. Usage guide: `x-docs/agent-api-usage-ac-implementation.md`. Re-ingest after code changes (`ac refresh` / `POST /api/projects/ac/refresh`, or `--deep`/`?deep=true` for a full field-level pass) before trusting query results. - **Tool priority: REST API first, then the `ac` CLI, then grep/Explore.** Query the REST endpoints diff --git a/README.md b/README.md index 1a59853..6feda70 100644 --- a/README.md +++ b/README.md @@ -245,7 +245,7 @@ The server exposes every query capability as REST endpoints under `http://localh `/flow-backward`, `/variables/{n}/field-flow`, `/variables/{n}/reads` and `/writes`, `/dynamic-calls/unresolved`, `/dynamic-calls/overrides`, `/refresh`, and more. The OpenAPI spec is served at `/q/openapi`. A full usage guide with response fields and semantics lives in [ -`x-docs/mcp-api-usage-ac-implementation.md`](x-docs/mcp-api-usage-ac-implementation.md). +`x-docs/agent-api-usage-ac-implementation.md`](x-docs/agent-api-usage-ac-implementation.md). --- diff --git a/ac-cli/src/main/resources/agenticcode.properties b/ac-cli/src/main/resources/agenticcode.properties index 8b7e45b..defe50b 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=176 +version=182 diff --git a/ac-code-server/src/main/resources/application.properties b/ac-code-server/src/main/resources/application.properties index a6d5f0f..12c332a 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 OpenAPI # info version (referenced below via property expression, not duplicated). -agenticcode.version=176 +agenticcode.version=182 # OpenAPI / Swagger UI (item 48) — the generated spec is the contract the web-UI TS client # is generated against. Served at /q/openapi (yaml/json); Swagger UI at /q/swagger-ui in dev. mp.openapi.extensions.smallrye.info.title=AgenticCode API diff --git a/ac-code-server/src/test/java/com/agenticcode/codeserver/api/InheritedFieldCallIT.java b/ac-code-server/src/test/java/com/agenticcode/codeserver/api/InheritedFieldCallIT.java index 7d20561..e5d9f50 100644 --- a/ac-code-server/src/test/java/com/agenticcode/codeserver/api/InheritedFieldCallIT.java +++ b/ac-code-server/src/test/java/com/agenticcode/codeserver/api/InheritedFieldCallIT.java @@ -14,11 +14,11 @@ import java.io.UncheckedIOException; import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; +import java.util.List; import java.util.Map; import static io.restassured.RestAssured.given; -import static org.hamcrest.Matchers.hasItem; -import static org.hamcrest.Matchers.not; +import static org.hamcrest.Matchers.*; import static org.junit.jupiter.api.Assertions.assertEquals; /** @@ -77,6 +77,19 @@ class InheritedFieldCallIT { } } """); + // A receiver that resolves to nothing at all: the marker for it can never be rewired, so it + // is the case the cleanup has to catch. It used to survive and be served from /callees. + write("DanglingLogic.java", """ + package p; + public class DanglingLogic { + public void work(Object o) { + somethingUndeclared.doIt(); + } + } + """); + // Item 118/B (lambda and catch parameters are bound names, not inherited fields) is covered by + // JavaParserTest#boundNamesAreNotTakenForInheritedFieldReceivers, not here: the cleanup deletes + // every marker, so an end-to-end assertion would pass whether or not the marker was created. given().contentType("application/json") .body(new ProjectResource.ProjectRequest(null, root.toString(), null, "java", null, null)) .when().post("/api/projects/" + PROJECT) @@ -110,20 +123,56 @@ class InheritedFieldCallIT { .body("items.name", hasItem("p.GrandChildLogic")); } + /** + * These two assertions replace a pair that tested {@code STARTS WITH 'field:'} and + * {@code hasItem("field:repo")}. The prefix constant carried a stray {@code U+0001}, so the real + * markers were named {@code field:*} — the tests were true because they matched nothing, + * while 202 markers survived in a real project and one was served from {@code /callees}. + * + *

Hence: match the marker anywhere in the name (a ':' cannot occur in a module name), and + * assert the underlying invariant separately — a module name never contains a control character. + * Either one alone can be satisfied by a name the author did not anticipate. + */ @Test - void placeholderMarkersDoNotSurviveEnrichment() { + void noMarkerSurvivesEnrichmentUnderAnyName() { try (Session session = driver.session()) { long leftovers = session.run( - "MATCH (m:MODULE {project: $p}) WHERE m.name STARTS WITH 'field:' RETURN count(m) AS c", + "MATCH (m:MODULE {project: $p}) WHERE m.name CONTAINS 'field:' RETURN count(m) AS c", Map.of("p", PROJECT)).single().get("c").asLong(); - assertEquals(0, leftovers, "the field:* receiver markers are scaffolding and must be cleaned up"); + assertEquals(0, leftovers, "the field: receiver markers are scaffolding and must be cleaned up"); } } + @Test + void noModuleNameContainsAControlCharacter() { + try (Session session = driver.session()) { + List odd = session.run( + "MATCH (m:MODULE {project: $p}) RETURN m.name AS n", Map.of("p", PROJECT)) + .list(r -> r.get("n").asString()).stream() + .filter(n -> n.chars().anyMatch(c -> c < 0x20)) + .toList(); + assertEquals(List.of(), odd, "a module name is an identity an agent passes back in a URL"); + } + } + + /** + * The leak was visible through the API, not only in the graph — which is why the graph-only + * assertions above are not enough on their own. {@code somethingUndeclared} resolves to nothing, + * so its marker is the one that used to survive. + */ + @Test + void anUnresolvableReceiverDoesNotLeakIntoCallees() { + List callees = given().when().get("/api/projects/" + PROJECT + "/modules/p.DanglingLogic/callees") + .then().statusCode(200) + .extract().jsonPath().getList("items.name", String.class); + assertEquals(List.of(), callees.stream().filter(n -> n != null && n.contains("field:")).toList(), + "internal scaffolding must never reach an API response"); + } + @Test void theMarkerIsNotExposedAsAModule() { given().when().get("/api/projects/" + PROJECT + "/modules?limit=100") .then().statusCode(200) - .body("name", not(hasItem("field:repo"))); + .body("name", not(hasItem(containsString("field:")))); } } 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 6876703..abbc2e6 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 @@ -698,7 +698,11 @@ public final class CypherQueries { public static final String RESOLVE_SIMPLE_NAME_REFERENCES = """ MATCH (ph:MODULE {project: $project}) WHERE ph.sourceFile = '' AND NOT ph.name CONTAINS '.' - AND NOT ph.name STARTS WITH 'field:' + // CONTAINS, not STARTS WITH: a ':' cannot occur in a Java or Natural module name, + // so this is equally sharp — but it also catches a marker written by an older + // ingest, which a refresh would otherwise never clear (placeholders have no + // sourceFile, so the per-file sweeps do not reach them). + AND NOT ph.name CONTAINS 'field:' MATCH (real:MODULE {project: $project, simpleName: ph.name}) WHERE real.sourceFile <> '' WITH ph, collect(DISTINCT real) AS candidates @@ -779,10 +783,18 @@ public final class CypherQueries { * {@link #RESOLVE_INHERITED_FIELD_RECEIVERS} could not resolve — the receiver may be a static * import, an outer-class field already handled at parse time (116a), or a type absent from the * project. Leaving them would put {@code field:partnerRepository} into the module namespace. + * + *

Matched with {@code CONTAINS}, deliberately. The prefix constant once carried a stray + * {@code U+0001}, so a {@code STARTS WITH 'field:'} predicate matched nothing at all: 202 markers + * survived in one project, 180 of them still wired, and {@code field:e} was served from + * {@code /callees}. The two guards written against that literal — here and in + * {@link #RESOLVE_SIMPLE_NAME_REFERENCES} — were both silently dead. A ':' cannot occur in a + * module name, so matching anywhere is just as precise and survives a marker written by any + * earlier build. */ public static final String DELETE_UNRESOLVED_FIELD_RECEIVERS = """ MATCH (ph:MODULE {project: $project}) - WHERE ph.sourceFile = '' AND ph.name STARTS WITH 'field:' + WHERE ph.sourceFile = '' AND ph.name CONTAINS 'field:' DETACH DELETE ph """; diff --git a/ac-parser-java/src/main/java/com/agenticcode/parserjava/JavaParser.java b/ac-parser-java/src/main/java/com/agenticcode/parserjava/JavaParser.java index 7d1870e..689d7f4 100644 --- a/ac-parser-java/src/main/java/com/agenticcode/parserjava/JavaParser.java +++ b/ac-parser-java/src/main/java/com/agenticcode/parserjava/JavaParser.java @@ -154,19 +154,36 @@ public final class JavaParser implements LanguageParser { * never collide with a real class name, and so the cleanup step can find every leftover. */ public static final String UNRESOLVED_FIELD_RECEIVER = "unresolvedFieldReceiver"; - public static final String UNRESOLVED_FIELD_RECEIVER_PREFIX = "field:"; + public static final String UNRESOLVED_FIELD_RECEIVER_PREFIX = "field:"; /** * @return whether {@code scope} is a bare lower-case identifier that this class does not declare — * i.e. it reads like a field rather than a type. Upper-case bare names are static calls and are * already handled; anything with a declared type resolved before we got here. */ - private static boolean isProbableFieldReceiver(Expression scope, Map declaredTypes) { + /** + * True when a call's receiver is most likely a field this class does not declare itself — i.e. + * one inherited from a supertype in another file (item 116b). + * + *

{@code locallyBound} is the exclusion that makes this usable: every name the enclosing + * callable binds, including the parameters of nested lambdas and {@code catch} clauses. Without + * it, {@code .map(e -> e.getX())} looked like an inherited field named {@code e} — one class in a + * real codebase produced 24 such markers, and none of them could ever resolve. + * + *

Deliberately not folded into {@code declaredTypes}: an implicit lambda parameter has type + * {@code UnknownType}, and feeding that to {@link #resolveReceiverClass} would turn a silent + * omission into a confident edge to a module named after a non-type. The two questions are + * separate — {@code locallyBound} answers "is this a field?", {@code declaredTypes} answers + * "which type is it?". + */ + private static boolean isProbableFieldReceiver(Expression scope, Map declaredTypes, + Set locallyBound) { if (!scope.isNameExpr()) { return false; } String name = scope.asNameExpr().getNameAsString(); - return !name.isEmpty() && Character.isLowerCase(name.charAt(0)) && !declaredTypes.containsKey(name); + return !name.isEmpty() && Character.isLowerCase(name.charAt(0)) + && !declaredTypes.containsKey(name) && !locallyBound.contains(name); } @Nullable @@ -242,7 +259,7 @@ public final class JavaParser implements LanguageParser { * {@code L} suffix); references to another constant in the same class (bare {@code NAME} or * {@code ThisClass.NAME}) are resolved transitively. */ - private static Map collectConstants(ClassOrInterfaceDeclaration type) { + private static Map collectConstants(TypeDeclaration type) { String className = type.getNameAsString(); Map initializers = new LinkedHashMap<>(); for (FieldDeclaration field : type.getFields()) { @@ -334,7 +351,7 @@ public final class JavaParser implements LanguageParser { * Resolves the table name from {@code @Entity(name = ...)} / {@code @Table(name = ...)}, * resolving a constant reference via {@code constants}, falling back to the class name. */ - private static String resolveTableName(ClassOrInterfaceDeclaration type, String className, + private static String resolveTableName(TypeDeclaration type, String className, Map constants) { for (String annotationName : List.of("Table", "Entity")) { @Nullable AnnotationExpr ann = annotation(type, annotationName).orElse(null); @@ -390,7 +407,7 @@ public final class JavaParser implements LanguageParser { return props; } - private static Optional annotation(ClassOrInterfaceDeclaration type, String name) { + private static Optional annotation(TypeDeclaration type, String name) { return type.getAnnotationByName(name); } @@ -745,15 +762,37 @@ public final class JavaParser implements LanguageParser { * parser cannot see (it parses one file) — that needs the enrichment stage that already resolves * the type hierarchy for {@code INJECTS}, and is tracked separately. */ - private static Map enclosingFieldTypes(ClassOrInterfaceDeclaration type) { - Deque outermostFirst = new ArrayDeque<>(); - for (ClassOrInterfaceDeclaration enclosing = type.findAncestor(ClassOrInterfaceDeclaration.class).orElse(null); + /** + * The type lexically enclosing {@code type}, of any kind — a class nested in a record or an enum + * counts. Item 119: written as a helper rather than {@code findAncestor(TypeDeclaration.class)} + * inline, because the wildcard makes the generic form unwieldy at each call site. + */ + + private static TypeFacts facts(TypeDeclaration type) { + return switch (type) { + case ClassOrInterfaceDeclaration c -> new TypeFacts(c.isInterface() ? "INTERFACE" : "CLASS", + c.isInterface(), c.getExtendedTypes(), c.getImplementedTypes()); + case EnumDeclaration e -> new TypeFacts("ENUM", false, List.of(), e.getImplementedTypes()); + case RecordDeclaration r -> new TypeFacts("RECORD", false, List.of(), r.getImplementedTypes()); + case AnnotationDeclaration a -> new TypeFacts("ANNOTATION", false, List.of(), List.of()); + default -> new TypeFacts("CLASS", false, List.of(), List.of()); + }; + } + + @Nullable + private static TypeDeclaration enclosingType(TypeDeclaration type) { + return type.findAncestor(TypeDeclaration.class).map(t -> (TypeDeclaration) t).orElse(null); + } + + private static Map enclosingFieldTypes(TypeDeclaration type) { + Deque> outermostFirst = new ArrayDeque<>(); + for (TypeDeclaration enclosing = enclosingType(type); enclosing != null; - enclosing = enclosing.findAncestor(ClassOrInterfaceDeclaration.class).orElse(null)) { + enclosing = enclosingType(enclosing)) { outermostFirst.addFirst(enclosing); } Map types = new HashMap<>(); - for (ClassOrInterfaceDeclaration enclosing : outermostFirst) { + for (TypeDeclaration enclosing : outermostFirst) { for (FieldDeclaration field : enclosing.getFields()) { for (VariableDeclarator variable : field.getVariables()) { types.put(variable.getNameAsString(), variable.getTypeAsString()); @@ -771,7 +810,7 @@ public final class JavaParser implements LanguageParser { * position ({@code super(XStep.class, …)}, {@code batchlet(refName(X.class))}). Targets are * deduped placeholder modules resolved to real modules by the finalize step. */ - private static void addWiringEdges(ClassOrInterfaceDeclaration type, AstNode typeNode, + private static void addWiringEdges(TypeDeclaration type, AstNode typeNode, Map referencedModules, TypeResolver types, List nodes, List edges) { for (FieldDeclaration field : type.getFields()) { @@ -816,6 +855,10 @@ public final class JavaParser implements LanguageParser { } } + private static boolean hasCdiScope(TypeDeclaration type) { + return type.getAnnotations().stream().anyMatch(a -> CDI_SCOPES.contains(a.getNameAsString())); + } + private static boolean isPlausibleEntityType(String type) { return !NON_ENTITY_RETURN_TYPES.contains(type) && !NON_DB_RECEIVER_TYPES.contains(type); } @@ -966,10 +1009,6 @@ public final class JavaParser implements LanguageParser { // DI + class-literal wiring edges (item J2) // ------------------------------------------------------------------------- - private static boolean hasCdiScope(ClassOrInterfaceDeclaration type) { - return type.getAnnotations().stream().anyMatch(a -> CDI_SCOPES.contains(a.getNameAsString())); - } - @Override public ParseResult parse(String sourceFile, String content) { List nodes = new ArrayList<>(); @@ -982,7 +1021,8 @@ public final class JavaParser implements LanguageParser { TypeResolver types = new TypeResolver(unit); - for (ClassOrInterfaceDeclaration type : unit.findAll(ClassOrInterfaceDeclaration.class)) { + for (TypeDeclaration type : unit.findAll(TypeDeclaration.class)) { + TypeFacts facts = facts(type); String className = type.getNameAsString(); // Item 117: the fully-qualified name IS the module's identity — a simple name does not // identify a class (nested @Nested classes, Builder, WorkingStorage: 8% of one real @@ -994,34 +1034,39 @@ public final class JavaParser implements LanguageParser { moduleProps.put("simpleName", className); moduleProps.put("fqn", fqn); // J3: distinguish interfaces from classes (interface -> implementation resolution). - moduleProps.put("isInterface", String.valueOf(type.isInterface())); - // Item 1: persisted module sub-kind for API filtering ("list all interfaces"). Enums - // and records aren't parsed as MODULE nodes at all yet (a larger, separate feature — - // this only classifies what JavaParser already handles: ClassOrInterfaceDeclaration). - moduleProps.put("moduleKind", type.isInterface() ? "INTERFACE" : "CLASS"); - // J1: tag repository classes with their managed entity so the enrichment step can map a - // repository method call to the entity's DB_TABLE. - @Nullable String repositoryEntity = repositoryEntityType(type); - if (repositoryEntity != null) { - moduleProps.put("repositoryEntity", repositoryEntity); - } - // J7: a project base class that passes Panache-ness one level up to its subclasses - // (e.g. AbstractPurRepository implements PanacheRepositoryBase). - // Tag it with its entity type parameter + own parameter list so the graph-side - // enrichment step can bind a concrete subclass's EXTENDS type argument to it. - @Nullable String panacheParam = panacheEntityTypeParam(type); - if (panacheParam != null) { - moduleProps.put("panacheEntityTypeParam", panacheParam); - moduleProps.put("typeParams", String.join(",", ownTypeParamNames(type))); - } - // J1b: a repository-named interface with no generic entity argument anywhere in sight - // (e.g. a project-specific IRiskRepository) — guess the entity from a method return type. - if (repositoryEntity == null && panacheParam == null - && type.isInterface() && isRepositoryReceiverName(className)) { - repositoryEntity = fallbackRepositoryEntity(type); + moduleProps.put("isInterface", String.valueOf(facts.isInterface())); + // Item 1 / item 119: persisted module sub-kind for API filtering ("list all interfaces"). + // CLASS | INTERFACE | ENUM | RECORD | ANNOTATION. + moduleProps.put("moduleKind", facts.kind()); + // The JPA/Panache heuristics below are class/interface notions — a record is never a + // Panache repository, and generalizing them would apply guesswork to types the assumption + // was never written for. + @Nullable String repositoryEntity = null; + if (type instanceof ClassOrInterfaceDeclaration cls) { + // J1: tag repository classes with their managed entity so the enrichment step can map + // a repository method call to the entity's DB_TABLE. + repositoryEntity = repositoryEntityType(cls); if (repositoryEntity != null) { moduleProps.put("repositoryEntity", repositoryEntity); } + // J7: a project base class that passes Panache-ness one level up to its subclasses + // (e.g. AbstractPurRepository implements PanacheRepositoryBase). + // Tag it with its entity type parameter + own parameter list so the graph-side + // enrichment step can bind a concrete subclass's EXTENDS type argument to it. + @Nullable String panacheParam = panacheEntityTypeParam(cls); + if (panacheParam != null) { + moduleProps.put("panacheEntityTypeParam", panacheParam); + moduleProps.put("typeParams", String.join(",", ownTypeParamNames(cls))); + } + // J1b: a repository-named interface with no generic entity argument anywhere in sight + // (e.g. a project-specific IRiskRepository) — guess the entity from a method return type. + if (repositoryEntity == null && panacheParam == null + && cls.isInterface() && isRepositoryReceiverName(className)) { + repositoryEntity = fallbackRepositoryEntity(cls); + if (repositoryEntity != null) { + moduleProps.put("repositoryEntity", repositoryEntity); + } + } } type.getJavadoc().ifPresent(jd -> { String firstLine = jd.getDescription().toText().lines() @@ -1047,14 +1092,14 @@ public final class JavaParser implements LanguageParser { // duplicate nodes that collide on the (type, name, sourceFile, project) merge key. Map referencedModules = new HashMap<>(); - for (ClassOrInterfaceType extended : type.getExtendedTypes()) { + for (ClassOrInterfaceType extended : facts.extended()) { AstNode superType = referencedModule(referencedModules, nodes, types.resolve(supertypeName(extended, className))); Map extendsProps = extendsTypeArgs(extended); edges.add(extendsProps.isEmpty() ? edge(EdgeType.EXTENDS, typeNode.id(), superType.id(), typeNode.startLine()) : edge(EdgeType.EXTENDS, typeNode.id(), superType.id(), typeNode.startLine(), null, extendsProps)); } - for (ClassOrInterfaceType implemented : type.getImplementedTypes()) { + for (ClassOrInterfaceType implemented : facts.implemented()) { AstNode interfaceType = referencedModule(referencedModules, nodes, types.resolve(supertypeName(implemented, className))); edges.add(edge(EdgeType.IMPLEMENTS, typeNode.id(), interfaceType.id(), typeNode.startLine())); } @@ -1064,7 +1109,8 @@ public final class JavaParser implements LanguageParser { // JPA entity: resolve the table name and link the class to its DB_TABLE. A Panache // active-record entity (extends PanacheEntity[Base]) is an entity even without @Entity. - boolean isEntity = annotation(type, "Entity").isPresent() || extendsPanacheEntity(type); + boolean isEntity = annotation(type, "Entity").isPresent() + || (type instanceof ClassOrInterfaceDeclaration cls && extendsPanacheEntity(cls)); boolean isMappedSuperclass = annotation(type, "MappedSuperclass").isPresent(); if (isEntity && !isMappedSuperclass) { String tableName = resolveTableName(type, className, constants); @@ -1112,6 +1158,50 @@ public final class JavaParser implements LanguageParser { } } + // Item 119: the state a record, an enum or an annotation type carries is not declared as a + // FieldDeclaration, so the loop above sees none of it. Without these three the types would + // be modules with an empty body — "analysed, nothing found" for a record DTO's components, + // which is the failure mode item 114 is about. + for (RecordDeclaration record : type instanceof RecordDeclaration r ? List.of(r) : List.of()) { + for (Parameter component : record.getParameters()) { + int line = component.getBegin().map(p -> p.line).orElse(typeNode.startLine()); + AstNode componentNode = node(NodeType.FIELD, component.getNameAsString(), sourceFile, + line, line, component.getTypeAsString(), null, + Map.of("recordComponent", "true")); + nodes.add(componentNode); + edges.add(edge(EdgeType.CONTAINS, typeNode.id(), componentNode.id(), line)); + fieldsByName.put(component.getNameAsString(), componentNode); + fieldTypes.put(component.getNameAsString(), component.getTypeAsString()); + } + } + for (EnumDeclaration enumeration : type instanceof EnumDeclaration e ? List.of(e) : List.of()) { + for (EnumConstantDeclaration constant : enumeration.getEntries()) { + int line = constant.getBegin().map(p -> p.line).orElse(typeNode.startLine()); + // The constant's own class body (a per-constant override) is not modelled as a + // separate module; its methods would need an identity no source-level name gives them. + AstNode constantNode = node(NodeType.CONSTANT, constant.getNameAsString(), sourceFile, + line, line, className, null, Map.of("enumConstant", "true")); + nodes.add(constantNode); + edges.add(edge(EdgeType.CONTAINS, typeNode.id(), constantNode.id(), line)); + fieldsByName.put(constant.getNameAsString(), constantNode); + } + } + for (AnnotationMemberDeclaration member : type.getMembers().stream() + .filter(AnnotationMemberDeclaration.class::isInstance) + .map(AnnotationMemberDeclaration.class::cast).toList()) { + int line = member.getBegin().map(p -> p.line).orElse(typeNode.startLine()); + Map memberProps = new HashMap<>(); + memberProps.put("annotationMember", "true"); + member.getDefaultValue().ifPresent(v -> memberProps.put("defaultValue", v.toString())); + // A FIELD, not a FUNCTION: the question asked of an annotation type is which attributes + // it carries, not which methods it declares. + AstNode memberNode = node(NodeType.FIELD, member.getNameAsString(), sourceFile, + line, line, member.getType().asString(), null, memberProps); + nodes.add(memberNode); + edges.add(edge(EdgeType.CONTAINS, typeNode.id(), memberNode.id(), line)); + fieldsByName.put(member.getNameAsString(), memberNode); + } + // Functions: methods + constructors (constructors are FUNCTION nodes named after the class). Map methods = new HashMap<>(); List> callables = new ArrayList<>(); @@ -1176,6 +1266,11 @@ public final class JavaParser implements LanguageParser { shadowed.add(v.getNameAsString()); declaredTypes.put(v.getNameAsString(), v.getTypeAsString()); })); + // Every *nested* parameter too — lambda and catch parameters. They are bound names + // like any local, but they are not VariableDeclarationExpr and not the callable's own + // parameter list, so both loops above miss them. Names only: an implicit lambda + // parameter has no usable declared type (see isProbableFieldReceiver). + callable.findAll(Parameter.class).forEach(p -> shadowed.add(p.getNameAsString())); addFieldAccessEdges(callable, callableNode, fieldsByName, shadowed, edges); @@ -1196,7 +1291,7 @@ public final class JavaParser implements LanguageParser { @Nullable String simpleTarget = resolveReceiverClass(scope, declaredTypes); @Nullable String targetClass = simpleTarget == null ? null : types.resolve(simpleTarget); // The DB-access candidate keeps the receiver as written (see extendsTypeArgs). - if (targetClass == null && isProbableFieldReceiver(scope, declaredTypes)) { + if (targetClass == null && isProbableFieldReceiver(scope, declaredTypes, shadowed)) { // Item 116b: the receiver names a field this class does not declare — almost // always one inherited from a supertype, which lives in another file the // parser never sees. Record the receiver's *name* against a placeholder so @@ -1242,6 +1337,27 @@ public final class JavaParser implements LanguageParser { return new ParseResult(nodes, edges); } + /** + * The handful of facts that differ between the four kinds of Java type declaration, resolved once + * so the ingest loop below can treat them uniformly. + * + *

Item 119: before this, the loop iterated {@code ClassOrInterfaceDeclaration} only, and enums, + * records and annotation types were not modules at all — 168 types in one real codebase, with + * every reference to them left dangling as a placeholder. They are modelled here because the three + * things that genuinely differ are exactly these: only a class/interface can {@code extends}, + * only an annotation type can do neither, and {@code isInterface} is a class/interface notion. + * Everything else ({@code getFields}, {@code getMethods}, {@code getConstructors}, + * {@code getAnnotations}, {@code getJavadoc}, {@code getFullyQualifiedName}) is common. + * + *

One deliberate omission: a record's compact canonical constructor. It is a + * {@code CompactConstructorDeclaration}, which — unlike every other member — does not extend + * {@code CallableDeclaration}, so it does not fit the callable machinery below and its body's + * calls stay invisible. One occurrence in the codebase that motivated this. + */ + private record TypeFacts(String kind, boolean isInterface, + List extended, List implemented) { + } + /** * True if {@code classExpr} is a direct argument of a method call, {@code new}, or {@code super()}/{@code this()}. */ @@ -1286,7 +1402,10 @@ public final class JavaParser implements LanguageParser { } // Types declared in this file — including nested ones, whose FQN is the enclosing chain // (a.b.Outer.Inner), which is exactly what made same-simple-name nested classes collide. - for (ClassOrInterfaceDeclaration declared : unit.findAll(ClassOrInterfaceDeclaration.class)) { + // Item 119: every kind of type declaration, not just class/interface — otherwise a + // reference to an enum or record declared in this very file cannot be qualified, which + // would be item 117 running backwards for exactly the types it just gained. + for (TypeDeclaration declared : unit.findAll(TypeDeclaration.class)) { declared.getFullyQualifiedName() .ifPresent(fqn -> declaredHere.put(declared.getNameAsString(), fqn)); } diff --git a/ac-parser-java/src/test/java/com/agenticcode/parserjava/JavaParserTest.java b/ac-parser-java/src/test/java/com/agenticcode/parserjava/JavaParserTest.java index 7055af9..dddc95e 100644 --- a/ac-parser-java/src/test/java/com/agenticcode/parserjava/JavaParserTest.java +++ b/ac-parser-java/src/test/java/com/agenticcode/parserjava/JavaParserTest.java @@ -10,6 +10,7 @@ import java.io.IOException; import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; +import java.util.List; import java.util.Map; import static org.junit.jupiter.api.Assertions.*; @@ -36,6 +37,12 @@ class JavaParserTest { && e.sourceId().equals(source.id()) && e.targetId().equals(target.id())); } + private static String prop(AstNode node, String key) { + Map props = node.properties(); + assertNotNull(props, "node " + node.name() + " has no properties"); + return String.valueOf(props.get(key)); + } + private static String readFixture(String name) throws IOException { Path path = Path.of("src/test/resources/fixtures/java", name); return Files.readString(path, StandardCharsets.UTF_8); @@ -277,4 +284,93 @@ class JavaParserTest { assertFalse(hasEdgeToNode(result, EdgeType.READS, shadowing, orderCountField)); assertFalse(hasEdgeToNode(result, EdgeType.WRITES, shadowing, orderCountField)); } + + /** + * Item 118/B: a lambda, catch or for-each variable is a bound name, not a field inherited from a + * supertype. Treating it as one produced a marker module per variable — 24 in a single class of a + * real codebase, none of them ever resolvable. + * + *

This has to be asserted here rather than through the API: the enrichment cleanup deletes + * every marker, so a leftover is invisible downstream and an end-to-end test of this would pass + * whether or not the marker was created. + */ + @Test + void boundNamesAreNotTakenForInheritedFieldReceivers() throws IOException { + LanguageParser.ParseResult result = + parser.parse("BoundNameReceivers.java", readFixture("BoundNameReceivers.java")); + + String prefix = JavaParser.UNRESOLVED_FIELD_RECEIVER_PREFIX; + assertEquals(List.of(prefix + "inherited"), + result.nodes().stream() + .filter(n -> n.type() == NodeType.MODULE && n.name().startsWith(prefix)) + .map(AstNode::name).sorted().toList(), + "only the genuinely undeclared receiver may be marked — not entry/element/ex"); + } + + /** + * The marker prefix is compared against in Cypher ({@code DELETE_UNRESOLVED_FIELD_RECEIVERS}), so + * a stray character in it disables the cleanup silently. That happened: a {@code U+0001} slipped + * in, 202 markers survived in one project and one was served from {@code /callees}. + */ + /** + * Item 119: enums, records and annotation types are modules, identified by their FQN like every + * other type. Before this they were not parsed at all — 168 types in one real codebase, and every + * reference to one of them stayed a dangling placeholder. + */ + @Test + void enumsRecordsAndAnnotationTypesAreModules() throws IOException { + LanguageParser.ParseResult result = parser.parse("TypeKinds.java", readFixture("TypeKinds.java")); + + assertEquals("ENUM", prop(findNode(result, NodeType.MODULE, "com.example.sample.TypeKinds.Status"), "moduleKind")); + assertEquals("RECORD", prop(findNode(result, NodeType.MODULE, "com.example.sample.TypeKinds.Money"), "moduleKind")); + assertEquals("ANNOTATION", prop(findNode(result, NodeType.MODULE, "com.example.sample.TypeKinds.Audited"), "moduleKind")); + // A class nested in a record is reached too — the enclosing-type walk is not class-only. + findNode(result, NodeType.MODULE, "com.example.sample.TypeKinds.Money.Formatter"); + } + + /** + * The state each kind carries is not a {@code FieldDeclaration}, so without explicit handling the + * modules above would have an empty body — a record DTO reporting no components is the + * "analysed, nothing found" answer item 114 is about. + */ + @Test + void recordComponentsEnumConstantsAndAnnotationMembersAreCaptured() throws IOException { + LanguageParser.ParseResult result = parser.parse("TypeKinds.java", readFixture("TypeKinds.java")); + + AstNode money = findNode(result, NodeType.MODULE, "com.example.sample.TypeKinds.Money"); + assertTrue(hasEdge(result, EdgeType.CONTAINS, money, "amount", NodeType.FIELD)); + assertTrue(hasEdge(result, EdgeType.CONTAINS, money, "currency", NodeType.FIELD)); + assertEquals("long", findNode(result, NodeType.FIELD, "amount").dataType()); + + AstNode status = findNode(result, NodeType.MODULE, "com.example.sample.TypeKinds.Status"); + assertTrue(hasEdge(result, EdgeType.CONTAINS, status, "OPEN", NodeType.CONSTANT)); + assertTrue(hasEdge(result, EdgeType.CONTAINS, status, "CLOSED", NodeType.CONSTANT)); + assertTrue(hasEdge(result, EdgeType.CONTAINS, status, "terminal", NodeType.FUNCTION)); + + AstNode audited = findNode(result, NodeType.MODULE, "com.example.sample.TypeKinds.Audited"); + assertTrue(hasEdge(result, EdgeType.CONTAINS, audited, "value", NodeType.FIELD)); + assertEquals("true", prop(findNode(result, NodeType.FIELD, "enabled"), "defaultValue")); + } + + /** + * An enum's or record's {@code implements} is a real edge — it feeds the CHA fan-out. + */ + @Test + void enumsAndRecordsCarryTheirImplementsEdges() throws IOException { + LanguageParser.ParseResult result = parser.parse("TypeKinds.java", readFixture("TypeKinds.java")); + + assertTrue(hasEdge(result, EdgeType.IMPLEMENTS, + findNode(result, NodeType.MODULE, "com.example.sample.TypeKinds.Status"), + "Comparable", NodeType.MODULE)); + assertTrue(hasEdge(result, EdgeType.IMPLEMENTS, + findNode(result, NodeType.MODULE, "com.example.sample.TypeKinds.Money"), + "Comparable", NodeType.MODULE)); + } + + @Test + void theMarkerPrefixIsPlainText() { + assertEquals("field:", JavaParser.UNRESOLVED_FIELD_RECEIVER_PREFIX); + assertFalse(JavaParser.UNRESOLVED_FIELD_RECEIVER_PREFIX.chars().anyMatch(c -> c < 0x20), + "a control character here silently disables every Cypher guard that matches on it"); + } } diff --git a/ac-parser-java/src/test/resources/fixtures/java/BoundNameReceivers.java b/ac-parser-java/src/test/resources/fixtures/java/BoundNameReceivers.java new file mode 100644 index 0000000..0e97cfd --- /dev/null +++ b/ac-parser-java/src/test/resources/fixtures/java/BoundNameReceivers.java @@ -0,0 +1,26 @@ +package com.example.sample; + +import java.util.List; + +/** + * Fixture for item 118/B: names bound by a lambda, a catch clause and an enhanced-for are not + * inherited fields, and must not produce an unresolved-field-receiver marker. + * + *

{@code inherited} is the counter-case: it is declared nowhere in this file, so it really is a + * candidate for a field from a supertype and must keep producing a marker. + */ +public class BoundNameReceivers { + + public void run(List items) { + items.forEach(entry -> entry.trim()); + for (String element : items) { + element.length(); + } + try { + items.clear(); + } catch (RuntimeException ex) { + ex.getMessage(); + } + inherited.doWork(); + } +} diff --git a/ac-parser-java/src/test/resources/fixtures/java/TypeKinds.java b/ac-parser-java/src/test/resources/fixtures/java/TypeKinds.java new file mode 100644 index 0000000..10323f8 --- /dev/null +++ b/ac-parser-java/src/test/resources/fixtures/java/TypeKinds.java @@ -0,0 +1,49 @@ +package com.example.sample; + +import java.util.List; + +/** + * Fixture for item 119: the three declaration kinds that were not modules at all — enum, record and + * annotation type — plus a class nested in a record, to prove the enclosing-type walk is not + * class-only either. + */ +public class TypeKinds { + + /** An enum with an interface, constants and a method. */ + public enum Status implements Comparable { + OPEN, + CLOSED; + + public boolean terminal() { + return this == CLOSED; + } + } + + /** A record with components, an interface and a normal method. */ + public record Money(long amount, String currency) implements Comparable { + + public String display() { + return amount + " " + currency; + } + + @Override + public int compareTo(Money other) { + return Long.compare(amount, other.amount()); + } + + /** Nested in a record — the enclosing-field walk must reach a record, not only a class. */ + static class Formatter { + } + } + + /** An annotation type with a member and a defaulted member. */ + public @interface Audited { + String value(); + + boolean enabled() default true; + } + + public List all() { + return List.of(Status.OPEN, Status.CLOSED); + } +} diff --git a/ac-ui/e2e/java-fqn.spec.ts b/ac-ui/e2e/java-fqn.spec.ts new file mode 100644 index 0000000..104c3d5 --- /dev/null +++ b/ac-ui/e2e/java-fqn.spec.ts @@ -0,0 +1,46 @@ +import {expect, test} from "@playwright/test"; + +/** + * Item 117 UI: a Java module's identity is its fully-qualified name, but the UI must not *show* it. + * + * The existing explorer spec runs against `upms` — Natural, whose names never contain a dot — so it + * passes either way and proves nothing here. These two run against a Java project, where the identity + * and the label genuinely differ. + * + * Depends on project `pur` being ingested with a post-117 build (`name` = FQN, `simpleName` set). + */ + +const FQN = "com.uniqagroup.common.base.AbstractLogic"; +const SIMPLE = "AbstractLogic"; + +/** + * The regression this guards: with `name` holding the FQN, an anchored pattern on the class itself + * matched nothing, and the list looked empty rather than wrong. + */ +test("the module filter accepts the simple class name as well as the FQN", async ({page}) => { + await page.goto("/p/pur"); + await expect(page.getByText(/\d{2,} modules/)).toBeVisible(); + + const filter = page.getByPlaceholder(/Filter name or file/); + + await filter.fill(`^${SIMPLE}$`); + await expect(page.getByText(/^1 modules$/)).toBeVisible(); + + // The fully-qualified form still resolves — matching the short name is an addition, not a swap. + await filter.fill(`^${FQN}$`); + await expect(page.getByText(/^1 modules$/)).toBeVisible(); + + // And the package path stays searchable, which is the whole point of keeping `name` in the filter. + await filter.fill("common\\.base\\."); + await expect(page.getByText(/^[2-9]\d* modules$/)).toBeVisible(); +}); + +/** The module opens by its identity (the FQN) but is labelled with the short name. */ +test("a Java module is addressed by FQN and displayed by simple name", async ({page}) => { + await page.goto(`/p/pur/m/${FQN}?tab=overview`); + + const heading = page.getByRole("heading", {name: SIMPLE, exact: true}); + await expect(heading).toBeVisible(); + // The identity is not lost — it is one hover away. + await expect(heading).toHaveAttribute("title", FQN); +}); diff --git a/ac-ui/src/api/names.ts b/ac-ui/src/api/names.ts new file mode 100644 index 0000000..4b65c0f --- /dev/null +++ b/ac-ui/src/api/names.ts @@ -0,0 +1,31 @@ +/** + * Display names for module identities. + * + *

Since item 117 a Java module's identity — the `name` every endpoint takes and returns — is its + * fully-qualified name (`com.example.OrderService`, `a.b.Outer.Inner` for a nested class). That is the + * right key and the wrong label: it does not fit a list row, a tree line or a graph node. + * + *

Only the module *list* carries `simpleName`; `digest`, `graph`, `callers`, `callees` and + * `call-tree` return the identity alone. So where the server offers the short form we use it, and + * everywhere else we derive it. The derivation was checked against the server on a real codebase: + * for all 4734 qualified modules of one project, the text after the last dot equals `simpleName`. + * + *

Natural module names never contain a dot (verified across 3587 upms modules), so this is a + * no-op for them — as it is for projects ingested before 117, whose names are still short. + */ + +/** The label to show for a module identity. Never use this as a key, a route or a request parameter. */ +export function shortName(name: string | undefined | null): string { + if (!name) return ""; + const dot = name.lastIndexOf("."); + return dot < 0 ? name : name.slice(dot + 1); +} + +/** + * Same, but prefers the short form the server itself derived. The server knows the cases a string + * rule cannot: a class in the default package, and a local/anonymous class for which JavaParser + * reports no qualified name at all — both keep their plain name rather than losing a segment. + */ +export function displayName(name: string | undefined | null, simpleName?: string | null): string { + return simpleName || shortName(name); +} diff --git a/ac-ui/src/api/schema.ts b/ac-ui/src/api/schema.ts index 60d9eb7..1311e3b 100644 --- a/ac-ui/src/api/schema.ts +++ b/ac-ui/src/api/schema.ts @@ -579,6 +579,7 @@ export interface paths { fields?: string; followWiring?: boolean; resolveInterfaces?: boolean; + sourceFile?: string; }; header?: never; path: { @@ -642,6 +643,7 @@ export interface paths { offset?: number; resolveInterfaces?: boolean; scope?: string; + sourceFile?: string; }; header?: never; path: { @@ -704,6 +706,7 @@ export interface paths { limit?: number; offset?: number; scope?: string; + sourceFile?: string; }; header?: never; path: { @@ -752,7 +755,9 @@ export interface paths { /** Entity Columns */ get: { parameters: { - query?: never; + query?: { + sourceFile?: string; + }; header?: never; path: { name: string; @@ -780,7 +785,7 @@ export interface paths { "application/json": components["schemas"]["ErrorResponse"]; }; }; - /** @description Module is an unresolved placeholder — its source is not ingested. */ + /** @description Ambiguous module name (AMBIGUOUS_NAME, with the candidate sourceFiles in details) or an unresolved placeholder whose source is not ingested (NOT_INGESTED). */ 409: { headers: { [name: string]: unknown; @@ -813,6 +818,7 @@ export interface paths { include?: string; limit?: number; offset?: number; + sourceFile?: string; }; header?: never; path: { @@ -841,7 +847,7 @@ export interface paths { "application/json": components["schemas"]["ErrorResponse"]; }; }; - /** @description Module is an unresolved placeholder — its source is not ingested. */ + /** @description Ambiguous module name (AMBIGUOUS_NAME, with the candidate sourceFiles in details) or an unresolved placeholder whose source is not ingested (NOT_INGESTED). */ 409: { headers: { [name: string]: unknown; @@ -870,7 +876,9 @@ export interface paths { /** Module Data Structures */ get: { parameters: { - query?: never; + query?: { + sourceFile?: string; + }; header?: never; path: { name: string; @@ -898,7 +906,7 @@ export interface paths { "application/json": components["schemas"]["ErrorResponse"]; }; }; - /** @description Module is an unresolved placeholder — its source is not ingested. */ + /** @description Ambiguous module name (AMBIGUOUS_NAME, with the candidate sourceFiles in details) or an unresolved placeholder whose source is not ingested (NOT_INGESTED). */ 409: { headers: { [name: string]: unknown; @@ -931,6 +939,7 @@ export interface paths { depth?: number; limit?: number; offset?: number; + sourceFile?: string; }; header?: never; path: { @@ -959,7 +968,7 @@ export interface paths { "application/json": components["schemas"]["ErrorResponse"]; }; }; - /** @description Module is an unresolved placeholder — its source is not ingested. */ + /** @description Ambiguous module name (AMBIGUOUS_NAME, with the candidate sourceFiles in details) or an unresolved placeholder whose source is not ingested (NOT_INGESTED). */ 409: { headers: { [name: string]: unknown; @@ -988,7 +997,9 @@ export interface paths { /** Module Digest */ get: { parameters: { - query?: never; + query?: { + sourceFile?: string; + }; header?: never; path: { name: string; @@ -1045,7 +1056,9 @@ export interface paths { /** Dispatch Table */ get: { parameters: { - query?: never; + query?: { + sourceFile?: string; + }; header?: never; path: { name: string; @@ -1073,7 +1086,7 @@ export interface paths { "application/json": components["schemas"]["ErrorResponse"]; }; }; - /** @description Module is an unresolved placeholder — its source is not ingested. */ + /** @description Ambiguous module name (AMBIGUOUS_NAME, with the candidate sourceFiles in details) or an unresolved placeholder whose source is not ingested (NOT_INGESTED). */ 409: { headers: { [name: string]: unknown; @@ -1105,6 +1118,7 @@ export interface paths { query?: { includeInherited?: boolean; kind?: string; + sourceFile?: string; }; header?: never; path: { @@ -1133,7 +1147,7 @@ export interface paths { "application/json": components["schemas"]["ErrorResponse"]; }; }; - /** @description Module is an unresolved placeholder — its source is not ingested. */ + /** @description Ambiguous module name (AMBIGUOUS_NAME, with the candidate sourceFiles in details) or an unresolved placeholder whose source is not ingested (NOT_INGESTED). */ 409: { headers: { [name: string]: unknown; @@ -1162,7 +1176,9 @@ export interface paths { /** Bulk Function Overrides */ get: { parameters: { - query?: never; + query?: { + sourceFile?: string; + }; header?: never; path: { name: string; @@ -1190,7 +1206,7 @@ export interface paths { "application/json": components["schemas"]["ErrorResponse"]; }; }; - /** @description Module is an unresolved placeholder — its source is not ingested. */ + /** @description Ambiguous module name (AMBIGUOUS_NAME, with the candidate sourceFiles in details) or an unresolved placeholder whose source is not ingested (NOT_INGESTED). */ 409: { headers: { [name: string]: unknown; @@ -1219,7 +1235,9 @@ export interface paths { /** Function Callers */ get: { parameters: { - query?: never; + query?: { + sourceFile?: string; + }; header?: never; path: { function: string; @@ -1277,7 +1295,9 @@ export interface paths { /** Function Overrides */ get: { parameters: { - query?: never; + query?: { + sourceFile?: string; + }; header?: never; path: { function: string; @@ -1306,7 +1326,7 @@ export interface paths { "application/json": components["schemas"]["ErrorResponse"]; }; }; - /** @description Module is an unresolved placeholder — its source is not ingested. */ + /** @description Ambiguous module name (AMBIGUOUS_NAME, with the candidate sourceFiles in details) or an unresolved placeholder whose source is not ingested (NOT_INGESTED). */ 409: { headers: { [name: string]: unknown; @@ -1345,6 +1365,7 @@ export interface paths { direction?: string; /** @description Cap on the number of nodes returned (BFS order). */ limit?: number; + sourceFile?: string; }; header?: never; path: { @@ -1393,7 +1414,9 @@ export interface paths { /** Payload */ get: { parameters: { - query?: never; + query?: { + sourceFile?: string; + }; header?: never; path: { name: string; @@ -1421,7 +1444,7 @@ export interface paths { "application/json": components["schemas"]["ErrorResponse"]; }; }; - /** @description Module is an unresolved placeholder — its source is not ingested. */ + /** @description Ambiguous module name (AMBIGUOUS_NAME, with the candidate sourceFiles in details) or an unresolved placeholder whose source is not ingested (NOT_INGESTED). */ 409: { headers: { [name: string]: unknown; @@ -1452,6 +1475,7 @@ export interface paths { parameters: { query?: { endLine?: number; + sourceFile?: string; startLine?: number; }; header?: never; @@ -1503,6 +1527,7 @@ export interface paths { parameters: { query?: { depth?: number; + sourceFile?: string; }; header?: never; path: { @@ -1531,7 +1556,7 @@ export interface paths { "application/json": components["schemas"]["ErrorResponse"]; }; }; - /** @description Module is an unresolved placeholder — its source is not ingested. */ + /** @description Ambiguous module name (AMBIGUOUS_NAME, with the candidate sourceFiles in details) or an unresolved placeholder whose source is not ingested (NOT_INGESTED). */ 409: { headers: { [name: string]: unknown; @@ -1563,6 +1588,7 @@ export interface paths { query?: { limit?: number; offset?: number; + sourceFile?: string; }; header?: never; path: { @@ -1591,7 +1617,7 @@ export interface paths { "application/json": components["schemas"]["ErrorResponse"]; }; }; - /** @description Module is an unresolved placeholder — its source is not ingested. */ + /** @description Ambiguous module name (AMBIGUOUS_NAME, with the candidate sourceFiles in details) or an unresolved placeholder whose source is not ingested (NOT_INGESTED). */ 409: { headers: { [name: string]: unknown; @@ -2759,6 +2785,7 @@ export interface components { sloc?: number; ingestStatus?: string; ingestDepth?: string; + simpleName?: string; }; NextAction: { method?: string; diff --git a/ac-ui/src/components/CallPanel.tsx b/ac-ui/src/components/CallPanel.tsx index e3e0fea..57aac10 100644 --- a/ac-ui/src/components/CallPanel.tsx +++ b/ac-ui/src/components/CallPanel.tsx @@ -1,4 +1,5 @@ import {useCalls} from "../api/hooks"; +import {shortName} from "../api/names"; interface Props { project: string; @@ -41,7 +42,7 @@ function CallList({ onClick={() => it.name && onOpen(it.name)} className="flex w-full items-center gap-2 rounded px-1.5 py-1 text-left text-sm hover:bg-neutral-100 dark:hover:bg-neutral-800" > - {it.name} + {shortName(it.name)} {it.edgeKind && ( diff --git a/ac-ui/src/components/CallTree.tsx b/ac-ui/src/components/CallTree.tsx index 2ebe9b6..5d2b670 100644 --- a/ac-ui/src/components/CallTree.tsx +++ b/ac-ui/src/components/CallTree.tsx @@ -1,5 +1,6 @@ import {useState} from "react"; import {useCalls} from "../api/hooks"; +import {shortName} from "../api/names"; interface Props { project: string; @@ -51,8 +52,9 @@ function TreeNode({ className={`truncate rounded px-1 text-left hover:bg-neutral-100 dark:hover:bg-neutral-800 ${ root ? "font-semibold" : "" }`} + title={name} > - {name} + {shortName(name)} {open && ( @@ -61,8 +63,8 @@ function TreeNode({ {!isLoading && children.length === 0 &&

no callees
} {children.map((child) => ancestors.includes(child) ? ( -
- ↻ {child} +
+ ↻ {shortName(child)}
) : ( {s.variableType}} {s.module && ( )} diff --git a/ac-ui/src/components/GraphView.tsx b/ac-ui/src/components/GraphView.tsx index dd734a1..3f92207 100644 --- a/ac-ui/src/components/GraphView.tsx +++ b/ac-ui/src/components/GraphView.tsx @@ -3,6 +3,7 @@ import Graph from "graphology"; import Sigma from "sigma"; import forceAtlas2 from "graphology-layout-forceatlas2"; import {fetchEgoGraph} from "../api/hooks"; +import {shortName} from "../api/names"; import type {EgoGraph} from "../api/client"; type Direction = "in" | "out" | "both"; @@ -32,7 +33,9 @@ function mergeEgo(graph: Graph, ego: EgoGraph, rootName: string) { if (!n.name) continue; const isRoot = n.name === rootName; graph.mergeNode(n.name, { - label: n.name, + // Node *key* stays the identity (item 117: the FQN); only the rendered label is shortened, + // or a Java graph is a wall of `com.example.…` and the nodes size to their package path. + label: shortName(n.name), sourceFile: n.sourceFile ?? "", unresolved: !!n.unresolved, color: isRoot ? COLOR.root : n.unresolved ? COLOR.unresolved : COLOR.resolved, @@ -190,7 +193,7 @@ export default function GraphView({project, moduleName, onOpen}: Props) { {selected && (
- {selected} + {shortName(selected)} ))}
diff --git a/ac-ui/src/components/ModuleTable.tsx b/ac-ui/src/components/ModuleTable.tsx index 93b8043..8e5f12f 100644 Binary files a/ac-ui/src/components/ModuleTable.tsx and b/ac-ui/src/components/ModuleTable.tsx differ diff --git a/ac-ui/src/components/ModuleView.tsx b/ac-ui/src/components/ModuleView.tsx index 157ee2a..310daa5 100644 --- a/ac-ui/src/components/ModuleView.tsx +++ b/ac-ui/src/components/ModuleView.tsx @@ -1,6 +1,7 @@ import {lazy, Suspense, useState} from "react"; import type {ModuleInfo} from "../api/client"; import {isModuleUnavailable, useIngestNeighborhood, useModuleContext, useRefreshModule} from "../api/hooks"; +import {displayName} from "../api/names"; import {StatusBadge} from "./StatusBadge"; import {ModuleOverview} from "./ModuleOverview"; import {SourceView} from "./SourceView"; @@ -79,7 +80,9 @@ export function ModuleView({
-

{module.name}

+

+ {displayName(module.name, module.simpleName)} +

{module.sourceFile}
diff --git a/ac-ui/src/routes/Explorer.tsx b/ac-ui/src/routes/Explorer.tsx index 0f3de2c..81aa20f 100644 --- a/ac-ui/src/routes/Explorer.tsx +++ b/ac-ui/src/routes/Explorer.tsx @@ -28,8 +28,13 @@ export function Explorer() { const all = data ?? []; const q = search.trim(); if (!q) return all; - // Treat the query as a case-insensitive regex over name/sourceFile; while it's an incomplete - // (invalid) pattern, fall back to a plain substring match so the list still filters as you type. + // Treat the query as a case-insensitive regex over name/simpleName/sourceFile; while it's an + // incomplete (invalid) pattern, fall back to a plain substring match so the list still filters + // as you type. + // + // Item 117: `name` is a Java module's fully-qualified name, so an anchored pattern on the class + // itself (`^AbstractLogic$`) stopped matching anything. Matching `simpleName` as well restores + // that without giving up search by package or by path. let re: RegExp | null = null; try { re = new RegExp(q, "i"); @@ -39,10 +44,12 @@ export function Explorer() { const lower = q.toLowerCase(); return all.filter((m) => { const name = m.name ?? ""; + const simple = m.simpleName ?? ""; const file = m.sourceFile ?? ""; return re - ? re.test(name) || re.test(file) - : name.toLowerCase().includes(lower) || file.toLowerCase().includes(lower); + ? re.test(name) || re.test(simple) || re.test(file) + : name.toLowerCase().includes(lower) || simple.toLowerCase().includes(lower) + || file.toLowerCase().includes(lower); }); }, [data, search]); @@ -167,6 +174,10 @@ export function Explorer() { + {/* Item 119: enums, records and annotation types became modules. */} + + + diff --git a/x-docs/agent-api-system-prompt.md b/x-docs/agent-api-system-prompt.md index 5149362..c595b12 100644 --- a/x-docs/agent-api-system-prompt.md +++ b/x-docs/agent-api-system-prompt.md @@ -110,7 +110,7 @@ LoC split** surfaced by `/loc` (`userExitLoc`/`userExitSloc` vs `generatedExclus consequence: when you read source to verify an API response for a Natural module, read the `generatedDir` copy (e.g. `generated_src/subprogram/WGEAGB0S.nat`) — the `user_exit` copy is a partial fragment and does not represent what was analysed. See the item-47 split in -`mcp-api-usage-ac-implementation.md`. +`agent-api-usage-ac-implementation.md`. ## 1. Pick a project diff --git a/x-docs/mcp-api-usage-ac-implementation.md b/x-docs/agent-api-usage-ac-implementation.md similarity index 98% rename from x-docs/mcp-api-usage-ac-implementation.md rename to x-docs/agent-api-usage-ac-implementation.md index 7f90de0..c6432a2 100644 --- a/x-docs/mcp-api-usage-ac-implementation.md +++ b/x-docs/agent-api-usage-ac-implementation.md @@ -132,6 +132,18 @@ GET /modules/OrderService/digest → 200 when unique, else 409 AMB Responses carry `simpleName` alongside `name` for display. The `?module=` and `?extends=` filters and the `ac` CLI take either form too; `--source-file` remains available on every module command. +**Which Java types are modules** (item 119). Classes, interfaces, **enums, records and annotation +types** — `moduleKind` is one of `CLASS | INTERFACE | ENUM | RECORD | ANNOTATION` (Natural adds +`PROGRAM | SUBPROGRAM | …`), and `?moduleKind=` filters on it. Their content is modelled the way each +kind carries it: a record's components and an annotation type's members are `FIELD`s (the latter with +`defaultValue` where declared), an enum's constants are `CONSTANT`s, and an enum's or record's +`implements` is a real edge — so a call against an interface fans out to an enum implementing it. + +Before 119 these three kinds were not parsed at all: `GET /modules/SomeEnum/digest` answered `404`, +and a record referenced from elsewhere stayed an unresolved placeholder (`409 NOT_INGESTED`). One +gap remains by design — a record's *compact* canonical constructor is not a function node, so calls +made in its body are invisible. + Natural is unaffected throughout: its module names are file stems, and colliding identities are skipped at ingest, so they are unique by construction (`upms` has zero ambiguous names, `pur` 163). diff --git a/x-docs/features.md b/x-docs/features.md index ba9db3f..8943e1c 100644 --- a/x-docs/features.md +++ b/x-docs/features.md @@ -4,6 +4,14 @@ Completed work, moved out of `x-docs/roadmap.md` (which now tracks only open items). Each entry records what was built; IDs are preserved from the roadmap (some IDs recur across sections — they are kept as-is for traceability). +> **The MCP surface no longer exists.** It was removed on 2026-08-04 (roadmap item 26) after +> intermittent session failures that were not fixable from this codebase; all 40 tools had a REST +> twin, so no capability was lost. **REST and the `ac` CLI are the only access paths.** +> +> Entries below still name MCP tools (`module_payload`, `McpQueryTools`, …). Those are kept as the +> historical record of what each feature shipped with — they are **not** a description of anything +> callable today. Where an entry reads as present tense, read "REST + CLI". + ## Metrics - [x] **47. Generated vs. user-exit LoC/SLoC split** (2026-07-13) — a project can declare a source @@ -364,7 +372,7 @@ automatic invalidation, remains open in the roadmap.) Natural **catalog metadata**, and `main-program`/`external-subprogram`/ `subprogram` are syntactically identical productions — no reliable source-level signal exists for these without the original catalog. See - `x-docs/mcp-api-usage-ac-implementation.md` step 0 for the heuristic's limits. + `x-docs/agent-api-usage-ac-implementation.md` step 0 for the heuristic's limits. ## Foundation (schema, health, errors, ingest CLI) @@ -960,7 +968,7 @@ automatic invalidation, remains open in the roadmap.) property, tagged on the existing call-site candidate); and a no-generic-entity fallback (repository interfaces with no resolvable generic type anywhere, e.g. a custom `IRiskRepository`, guess the entity from a declared method's return type). - See `x-docs/mcp-api-usage-ac-implementation.md` for semantics/limits. + See `x-docs/agent-api-usage-ac-implementation.md` for semantics/limits. ## Project model & ingest endpoints @@ -1586,7 +1594,7 @@ question the graph already had the data for. matching annotation, backed by a new generic `annotations` property captured at parse time on every such node (independent of any annotation's own specific interpretation elsewhere, e.g. `@Entity`/`@Query`). See - `x-docs/mcp-api-usage-ac-implementation.md` section 7-8 for usage. + `x-docs/agent-api-usage-ac-implementation.md` section 7-8 for usage. - [x] **31. Resolve inherited `REFERENCES`/wiring edges on concrete subclasses** (found 2026-07-07/08, PUR `pur-batch` re-evaluation, done 2026-07-09) — `call_tree`/ @@ -1701,7 +1709,7 @@ question the graph already had the data for. the validation can't drift between transports. New `McpToolsIT` drives the whole pipeline through tools only (connect → `ingest_all` → `list_modules`/`list_projects` + a `PROJECT_NOT_FOUND` error case); all 58 REST ITs still green after the - resolver refactor. Docs: `mcp-api-usage-ac-implementation.md` gained an "Access via MCP" + resolver refactor. Docs: `agent-api-usage-ac-implementation.md` gained an "Access via MCP" section. - [x] **30. Version number in startup log / API / MCP** (done 2026-07-08) — @@ -2242,7 +2250,7 @@ its own; `McpSupport` only serialized service results into the same JSON the RES **Docs & rules updated.** `CLAUDE.md` (sync rule now REST + `ac-cli`; tool priority now REST → CLI → grep/Explore; architecture diagram, project structure, ADR table), `README.md` (architecture diagram, tech stack, endpoint list, "Agent usage" section replacing "MCP"), `x-docs/agent-api-system-prompt.md` -("Access via MCP" section and its REST↔tool mapping table dropped), `x-docs/mcp-api-usage-ac-implementation.md` +("Access via MCP" section and its REST↔tool mapping table dropped), `x-docs/agent-api-usage-ac-implementation.md` (tool names replaced by their endpoint paths throughout; filename kept to avoid breaking ~6 inbound references), `x-docs/agenticcode-ueberblick.md`, `x-docs/presentation.md`, `prompts/CLAUDE.md`, both `prompts/*-deep-api-audit.md`, and `.claude/settings.local.json`. Historical MCP mentions in this file diff --git a/x-docs/roadmap.md b/x-docs/roadmap.md index 48288bd..1900fc4 100644 --- a/x-docs/roadmap.md +++ b/x-docs/roadmap.md @@ -321,6 +321,91 @@ before. The parked "parallel parse phase" idea was implemented 2026-07-18 (item ## Known bugs +- [x] **119. Enums, records and annotation types are modules** (found 2026-08-06 in the `pur` + source-vs-API cross-check; **done 2026-08-06**) + + The Java parser iterated `ClassOrInterfaceDeclaration`. Everything else was invisible — in `pur` + **76** annotation types, **67** enums and **25** records, 168 types with no `MODULE` node at all + (plus 384 `package-info.java`, correctly ignored). `GET /modules/BatchParam/digest` answered `404`; + a record referenced from another file stayed an unresolved placeholder and answered + `409 NOT_INGESTED`. The API did not lie — item 107 saw to that — but a whole category of type was + unanalysable. + + The loop now iterates `TypeDeclaration`. Exactly three things differ between the four kinds, and + they are resolved once in a `TypeFacts` adapter instead of scattering `instanceof` through a + 250-line body: only a class/interface can `extends`, only an annotation type can do neither, and + `isInterface` is a class/interface notion. `moduleKind` gains `ENUM | RECORD | ANNOTATION`. + + **What the probe corrected in the plan.** Rather than reason about the JavaParser API I ran a + parser over a fixture with all four kinds, and two assumptions were wrong: + - A record's *compact* canonical constructor (`Point { … }`) is **not** returned by + `getConstructors()` — `CompactConstructorDeclaration` does not extend `CallableDeclaration` and so + does not fit the callable machinery. Left uncovered deliberately (1 occurrence in `pur`), and + named here because the calls in its body stay invisible. + - An annotation type's `String value();` is **not** a method — `getMethods()` returns none. Building + it as planned would have given 76 annotation types a module node with an empty body: "analysed, + nothing found" again. Members are modelled as `FIELD`s with `defaultValue`, because the question + asked of an annotation is which attributes it carries. + + Record components and enum constants are captured for the same reason — a record DTO reporting zero + fields is worse than no answer. The `TypeResolver` and the enclosing-type walk had to follow, or a + reference to an enum declared in the same file would have been unqualifiable — item 117 running + backwards for precisely the types it just gained. + + Kept class/interface-only on purpose: the JPA/Panache repository heuristics. A record is not a + Panache repository, and generalizing guesswork to types it was never written for produces confident + wrong answers rather than silence. + + **Watched, not asserted:** the CHA fan-out grows, since an enum implementing a project interface is + now a real implementation target. And 168 additional types can make a previously unique + `simpleName` ambiguous, so a short name that used to work may now answer `409` — the honest answer + under 115, but a visible change. + +- [x] **118b. A control character in one constant disabled two guards, and the tests that should have + caught it asserted the same wrong literal** (found 2026-08-06 in the `pur` cross-check; + **done 2026-08-06**; both regressions from 116b) + + **A — the marker leak.** `UNRESOLVED_FIELD_RECEIVER_PREFIX` contained a stray `U+0001`: + + ``` + JavaParser.java:157 = ".field:"; + hex: 3d 20 22 01 66 69 65 6c 64 3a 22 3b + ``` + + So the parser wrote `field:x` while both Cypher guards matched `STARTS WITH 'field:'` — the + cleanup in `DELETE_UNRESOLVED_FIELD_RECEIVERS` and the exclusion in + `RESOLVE_SIMPLE_NAME_REFERENCES`. Neither ever matched. In `pur`, 202 marker nodes survived, 180 of + them still wired with 850 edges, and the scaffolding was served from the public API: + + ``` + GET /modules/…KundeService/callees → { "name": "field:e", … } + ``` + + **Why it survived review and tests.** The two ITs written to guard exactly this checked + `m.name STARTS WITH 'field:'` and `not hasItem("field:repo")` — the same wrong literal. They were + true because they matched nothing. Code and test were wrong in the same way, so the test could not + see the bug it existed for. + + Fixed at the source, and both predicates now match `CONTAINS 'field:'`: a ':' cannot occur in a Java + or Natural module name (verified across all four projects), so it is equally sharp — and it also + clears markers left by an earlier build, which a refresh would never reach otherwise (placeholders + have no `sourceFile`, so the per-file sweeps do not touch them). + + **B — lambda parameters taken for inherited fields.** `isProbableFieldReceiver` asked "lower-case + and not in `declaredTypes`?". Lambda and `catch` parameters are in neither the callable's parameter + list nor its `VariableDeclarationExpr`s, so `.map(e -> e.getX())` looked like an inherited field + named `e` — 24 markers from one class alone, none of them ever resolvable. The bound names now go + into the existing `shadowed` set. Deliberately *not* into `declaredTypes`: an implicit lambda + parameter has type `UnknownType`, and feeding that to the receiver resolver would turn a silent + omission into a confident edge to a module named after a non-type. + + **Both fixes proven by reverting them.** With A restored to its broken form, three assertions fail — + including `field:somethingUndeclared` reaching `/callees`. The first B test I wrote was *itself* + vacuous: with A fixed the cleanup deletes every marker, so nothing is observable end-to-end. It moved + to `JavaParserTest`, where reverting B turns it red with `[field:entry, field:ex, field:inherited]`. + The new assertions match the marker anywhere in the name and, separately, assert the invariant that + was actually violated — no module name contains a control character. + - [x] **118. The Java DB resolvers scanned the project once per candidate row** (found 2026-08-06 while watching a `pur` refresh; **done 2026-08-06**; regression from [117]) @@ -417,9 +502,44 @@ before. The parked "parallel parse phase" idea was implemented 2026-07-18 (item Also unified on the way: `declaredIn` is a display label in every producer (column metadata and both function queries), i.e. the short name. Two of the three had drifted to the identity. - **Not done:** the web UI. Its TypeScript client is generated from the running server's OpenAPI, so - `simpleName` cannot be surfaced until a deploy exists to generate against. The CLI needs no change — - it prints the response verbatim. + **UI (done 2026-08-06, after the deploy that unblocked the codegen).** The client is generated from + the running server's OpenAPI, so this had to wait for a build that knows `simpleName`. Regenerating + pulled in exactly the 115/117 contract and nothing else: 18 `sourceFile` query params, 11 reworded + 409 descriptions, 1 `simpleName`. + + The find was that this is **not** a cosmetic task. `Explorer.tsx` filters the module list by regex + over `name`, and `name` is now the FQN — so an anchored pattern on the class itself returned + nothing: + + ``` + ^AbstractLogic$ → 0 Treffer (name = com.uniqagroup.common.base.AbstractLogic) + ``` + + The existing explorer spec runs against `upms` (Natural, no dots in any name) and stayed green + throughout. The filter now also matches `simpleName`; the package path and the FQN remain + searchable. + + Display: eight sites render the short name with the identity in the `title` — module list, module + header, graph node labels **and** the selected-node chip, callers/callees, call tree (including its + cycle markers), impact list, dataflow steps. Identity is untouched everywhere it matters: routing, + query keys, `key=`, requests, and the graphology node key. Where the server sends `simpleName` it + wins over the client-side rule, because it knows the cases a string rule cannot — a class in the + default package, or a local class for which JavaParser reports no qualified name. + + The derivation ("text after the last dot") was checked against the server rather than assumed: for + all **4734** qualified modules of `pur`, it equals `simpleName` — 0 divergences; and 0 of 3587 + `upms` module names contain a dot, so Natural is provably unaffected. Worth noting the graph is + currently mixed-generation (only `pur` is re-ingested post-117), which is what makes the + prefer-then-derive fallback necessary rather than nice. + + **Accepted loss:** a nested class shows as `Inner`, so two `Outer.Inner` in different outers look + alike in a list. The FQN is one hover away and the list also shows the source file. + + New e2e spec `java-fqn.spec.ts` (the only one that runs against a Java project) — and it was + verified to actually catch the regression by reverting the filter fix and watching it go red. + `tsc --noEmit` + `vite build` clean; Playwright 21 passed, 1 pre-existing failure in + `identifier-popover.spec.ts` (expects `line=323`, gets `324` — reproduced with all UI changes + stashed, so it predates this work and belongs to the upms data, not the UI). - [x] **115. Module endpoints address Java classes by simple name and silently merge distinct classes that share one** (found 2026-08-06, `pur` source-vs-API cross-check; **done 2026-08-06**) @@ -1115,7 +1235,7 @@ wrong answer, found by the 2026-07-17 `VMULTMN4` audit.)* (genuine incoming CALLNAT/inheritance only). `context`/`digest` (both call `callers(…, null)`) inherit the clean view; `scope=internal` still exposes function→function PERFORM wiring; callees unchanged. Covered by `ModuleCallersSelfLoopIT` (fails 2/3 before the fix). MCP `callers` tool - description + REST endpoint doc + `mcp-api-usage-ac-implementation.md` updated. + description + REST endpoint doc + `agent-api-usage-ac-implementation.md` updated. *(Supersedes the earlier "Not a bug (verified): `context.callers` includes internal PERFORM callers — noisy but accurate" note.)* Ego-graph `direction=in` for `WGEAGB0S` now returns its dynamic callers `W-LST-N0`/`W-MNT-N0` (item 75). Payload `direction` is always `REQUEST` for PDA-derived contracts (a @@ -1514,7 +1634,7 @@ lands in both ingest tiers at once.)* (`substrPos IS NULL`). Precedence honoured by guarding the fold against `:DynamicCallOverride` sites plus a `delete-folded-overridden-dynamic-callnat` step before `apply-manual`. Characterization ITs in `DynamicCallnatFoldIT` (fold resolves `YABALKEY`+`GN0@6/3` → `YABALGN0`; manual override wins); - full dynamic-callnat regression 89/89 green. Docs in `mcp-api-usage-ac-implementation.md`. + full dynamic-callnat regression 89/89 green. Docs in `agent-api-usage-ac-implementation.md`. - [x] **26. MCP session reliability — RESOLVED BY REMOVAL (2026-08-04)** (investigated 2026-07-07, reproduced 2026-08-02, never fixed). `mcp__agenticcode__*` calls intermittently — and in the