This commit is contained in:
Ingo Schnabel
2026-08-27 18:24:28 +02:00
parent 2732d69bd1
commit 039ff1e176

View File

@@ -36,6 +36,10 @@ tracks only items that are still open.
> **Items 125–130 are high priority** (added 2026-08-18, see the section directly below), and
> **item 131** (added 2026-08-19) reopens item 103's silent-truncation defect on the three `search/*`
> endpoints — it produced a wrong audit finding in `pur`;
> **items 141-143 are high priority** (added 2026-08-27: comments and commented-out code are not
> indexed, so the documented Natural->Java counterpart lookup returns a false "not yet reengineered";
> a dead Natural subroutine is indistinguishable from a live one; and the cross-project counterpart
> relation has no edge) with items 144-150 filed alongside them at normal priority;
> all other remaining open items are postponed, revisit when prioritised.
## High priority — Agent API gaps found in the UPMS→PUR reengineering (2026-08-18)
@@ -501,6 +505,151 @@ its probe was not ambiguous, so the answer had been correct all along (see the r
then consider a narrower default (imports + annotations only, declared types behind a flag), or
emitting mentions only in the deep tier rather than in the Tier-1 scan.
## High priority — comments, dead code and the counterpart relation (2026-08-27)
Three gaps reported after the `VermittlerController` reengineering round (nine UPMS services, four of
them built in one session). They share one root: **the graph holds structure, and the UPMS→PUR work
lives half in the things that are not structure** — Natural comments, commented-out code, and the
Java↔Natural correspondence that is recorded only as a Javadoc convention. All probes below were run
2026-08-27 against a freshly refreshed graph (`upms` ingested 08:37Z, `pur` the same morning) and are
reproducible as written.
Item **141** is the one that makes the API return a *wrong* answer rather than a missing one, which is
why it leads the list.
- [ ] **141. Comments and commented-out code are not indexed — so the documented way to find a Java
counterpart returns a confident false negative**
**Symptom, part one: the counterpart lookup.** The consuming project's convention (its `CLAUDE.md`
§3) is that a reengineered web service carries its Natural origin in a Javadoc block, and that the
way to go Natural→Java is to search the program name in `pur`, where *"an empty result means not yet
reengineered"*. Measured:
```
GET /pur/search/value?value=JX0034N0&contains=true → 1 hit, MultiTableImportJob.java:33
GET /pur/search/value?value=WPARTX0S&contains=true → []
GET /pur/search/value?value=WEXKYX0S&contains=true → []
```
The batch job is found because `PROGRAM_IDENTIFIER = "JX0034N0"` is a **string literal**. `WPARTX0S`
and `WEXKYX0S` are **fully reengineered** — `PartnerController` and its logic classes have existed
for months — but their origin is recorded only in a Javadoc block:
```java
/**
* ServiceEndpoint: partner.update.partnercs.Update
* UPMSFunction: com.uniqagroup.upms.partner.svc.esp.UpmsPartnerCsUpdate
* UpmsObject: PartnerCs, UpmsAdapter: Update
*/
```
Comments are not nodes, so the search cannot see it. The API therefore answers *"not yet
reengineered"* — the exact wording the consuming project derives from an empty result — for **every
web service that has already been reengineered**. That is not a missing answer; it is the wrong one,
delivered with no signal, on the single question every reengineering job opens with. The only reason
it has not yet caused a duplicate implementation is that the agent happened to distrust it.
**Symptom, part two: the semantics of `upms` live in the comments.** Natural source in this corpus
carries its change history, its business caveats and its disabled logic as comment text:
```
GET /upms/search/value?value=Bug%20266&contains=true → []
```
while `WAGNTX0S.nat:21` reads `* #01 09.05.07 VOVBJ03 Bug 266`, and lines 22-23 record two
regenerations with their ticket numbers. The `#01`…`#05` change markers correlate to `--> #04` /
`<-- #04` blocks that delimit which statements a given change introduced — often the only record of
*why* a branch exists. None of it is queryable. Every such question falls back to reading the file,
which in the consuming project is explicitly the exception path and, for the Java side, blocked by a
hook.
**Proposed shape.** A `COMMENT` node per contiguous comment block, carrying `sourceFile`,
`startLine`, `endLine`, `text`, and an edge to the nearest following declaration (module, function,
field) — so `/modules/{name}/comments` answers "what does this module's header say" and
`search/value` reaches comment text like any other content. Three deliberate points:
* **`search/value` must keep the two kinds separable.** A comment hit and a code hit are not the
same evidence. Suggest `kind: "COMMENT"` on the existing row shape (it already discriminates
`ASSIGNMENT`/`NODE`) rather than a fourth search endpoint, plus a switch for callers that want
today's behaviour. Folding comments into the default result set silently would move every existing
completeness count — item 131's lesson.
* **Cost must be measured before it is defaulted on.** Natural in this corpus is comment-dense
(`WAGNTX0S.nat` is ~25% comment lines in its header alone), and item 128 already showed a 3-5×
deep-refresh cost for one new edge family. Measure persist time and store growth on `upms` before
deciding whether this belongs in Tier 1 or in the deep tier only.
* **Javadoc is structured, plain comments are not.** The `ServiceEndpoint:` / `UPMSFunction:` /
`UpmsObject:` block above is a key-value list. Parsing it into properties is item 143's business;
141 only has to make the *text* reachable, and 143 should not be blocked waiting for it.
- [ ] **142. `functions` cannot tell a live subroutine from a dead one — an auskommentierter `PERFORM`
is invisible**
**Symptom.**
```
GET /upms/modules/WAGNTX0S/functions → 40+ rows, every one { name, declaredIn, sourceFile,
viaCopycode, startLine, endLine, kind: null }
```
There is no field saying whether anything still performs this subroutine. In Natural of this age,
logic is not deleted — it is commented out, usually only at the `PERFORM` site, leaving the
`DEFINE SUBROUTINE` block fully intact and fully parseable. The graph sees a declared function with
a body and reports it as one.
**Why this is the expensive one.** The consuming project names it as such in its own instructions:
*"A subroutine whose `PERFORM` is commented out is dead; treating it as live is a common and serious
error."* The failure mode is not a wrong query result the agent can notice — it is a subroutine
faithfully reengineered into Java, reviewed, tested and shipped, implementing behaviour the legacy
system stopped executing years ago. Nothing downstream catches it, because the Java is a correct
translation of code that really is there.
**Proposed fix.** Two derived properties on the `FUNCTION` row: `performSites` (count of live
`PERFORM`/`CALLNAT` references reaching it) and `live` (`performSites > 0`, or the module's entry
point). A dead subroutine is then one field on a response the agent already fetches, instead of a
file read it has to remember to do. Note the ordering dependency: counting *live* sites means
knowing which `PERFORM`s are commented out, which is item 141's parse work — but only the cheap half
of it (recognising a comment line, not indexing its text), so 142 can ship on that alone if 141's
cost measurement goes against a full comment index.
Report `live: null` rather than `true` where the analysis cannot decide — a `PERFORM` behind an
unresolved dynamic dispatch (item 145), for instance. A false `true` restores exactly the failure
this item exists to remove.
- [ ] **143. The Natural↔Java↔caller correspondence has no edge, and "what is not reengineered yet"
cannot be asked**
**Symptom.** Three projects hold three views of one service, and nothing joins them:
| project | holds | how the link is recorded |
|---|---|---|
| `upms` | `WAGNTX0S.nat` | — |
| `pur` | `AgentProvisionUpdateLogic` | `UpmsObject:` Javadoc block (invisible, item 141) |
| `app` | `UpmsAgentUpdate` | named in the same Javadoc block |
Answering *"what is the counterpart of this"* today means a `search/value` per project, a guess at
the naming convention, and — for web services — a fallback to reading files, because of 141. It is
the first question of every job, and it costs several calls plus one known-unreliable heuristic.
**The question that cannot be asked at all.** *"Which `upms` web-service programs have no `pur`
counterpart yet?"* is the project's central planning question — what is left to do — and there is no
formulation of it against the API. It is currently answered by a human keeping a list.
**Proposed shape.** Parse the two conventions the codebase already follows — `PROGRAM_IDENTIFIER` /
the `// XXXXXXXX.nat` comment, and the `ServiceEndpoint:`/`UPMSFunction:`/`UpmsObject:` Javadoc
block — into a first-class cross-project edge:
```
pur:AgentProvisionUpdateLogic --REENGINEERED_FROM--> upms:WAGNTX0S
pur:AgentProvisionUpdateLogic --SERVES--> app:UpmsAgentUpdate
GET /upms/modules/WAGNTX0S/counterparts → the pur and app sides, with the evidence line
GET /upms/counterparts?missing=true → the backlog, as data
```
**Three things to get right.**
* **This is the first edge that crosses a project boundary.** Every existing query is
project-scoped, and the traversal queries (`callers`, `callees`, `call-tree`) must not start
following it. Item 128's `MENTIONS`-vs-`REFERENCES` decision is the precedent, and the reason it
was the right one.
* **The evidence must ride along.** Return the `sourceFile:lineNo` of the Javadoc block or of the
constant that produced the edge. A correspondence asserted without its evidence is unusable in a
project whose reports require file+line for every claim.
* **A missing counterpart is not the same as an unparseable one.** `missing=true` must distinguish
"no `pur` module claims this program" from "a module claims it in a form the parser did not
recognise", or the backlog list silently pads itself with the parser's own gaps.
## Web UI — code understanding & navigation
A React/TypeScript web UI (`ac-ui/`) for navigating the AgenticCode graph to
@@ -2721,3 +2870,110 @@ reproduced the bug.)*
server surface was removed entirely** rather than debugged — see "MCP surface removed" in
`x-docs/features.md`. REST + `ac` CLI are now the only access paths; all 40 former tools had a REST
twin, so no capability was lost.
## Smaller gaps from the same reengineering round (2026-08-27)
Normal priority. Same session as items 141-143, same probe date; none of these produced a wrong
answer, they cost calls or forced a file read. Three candidates from the original list were dropped
after probing and are recorded at the end, so nobody re-files them.
- [ ] **144. `db-accesses` is table-granular — "who writes this column" cannot be asked**
`db-accesses?depth=N` reports `READS`/`WRITES`/`DECLARES` per table. The expensive defect of this
session was one column deep: `VERSVW_AGNT_COMI` is a periodic group bound to a version through
`V_ID_REC`, and the historisation base class wrote the version row without carrying the group
forward — so every agent change silently dropped the commission schemes. Both sides *appear* in
`db-accesses`: the base class writes `VERSVW_AGENT`, some other class writes `VERSVW_AGNT_COMI`. At
table granularity there is nothing to notice.
A `?columns=true` variant returning `{table, column, mode, via, sourceFile, lineNo}` — or
`/db-tables/{name}/writers` from the table side — would have made "which code writes `V_ID_REC`"
a single call. The Java side is derivable without new parsing: `@Column` names are already stored
(item 130 resolves constant references for `@Path` the same way), and the entity→table mapping is
already the `DECLARES` row.
- [ ] **145. `call-tree` does not say how many unresolved dynamic `CALLNAT`s are inside the subtree it
just returned**
The response carries `sourceFiles`, `items` and `truncated` — so a traversal cut by the budget is
honest, which item 67 fixed. A traversal that is *complete as far as the graph knows* but crosses
three `CALLNAT #SUBPROGRAM` sites is not distinguishable from one that crosses none.
`/dynamic-calls/unresolved` lists every open site project-wide (`upms`: `BGARMFN0:1213`,
`BGARMSN0:657`, …), but nothing correlates that list with a given call tree, so checking costs a
second call plus a manual intersection against `sourceFiles` — and only an agent who already
suspects the problem will make it.
Suggest an `unresolvedDynamicCalls` count (and the sites) alongside `truncated`. Same principle:
a subtree that looks complete and is not is worse than one that admits it was cut.
- [ ] **146. `flow-forward` / `field-flow` stop at the `CALLNAT` parameter boundary**
Natural passes data positionally: `CALLNAT 'YXXXMN0' #A #B #C` binds to the callee's
`DEFINE DATA PARAMETER` list by position. The mapping is mechanical and the graph has both halves —
the call site's argument list and the callee's parameter declaration — but nothing joins them, so
a field's life ends at the call. Following a value through a three-deep `CALLNAT` chain is a manual
positional count per hop, done by reading both files, and it is where the reengineering evidence
chain actually lives ("which request field ends up in this column").
Item 68 already introduced the derived `CALLS_MODULE` edge to make `field-flow` cross subroutine
boundaries; this is the same class of derivation one level out.
- [ ] **147. Test coverage is not graph data**
*"Which endpoints of this controller have an active test, and which only a `@Disabled` stub?"* was a
literal task this session, and it was answered by reading the test class top to bottom. Everything
needed is already in the graph after item 128: `@Test`/`@Disabled` are annotations
(`search/annotation`), the test→subject relation is a `CALL` or `MENTIONS` edge, and item 130 knows
which method serves which REST path.
Something like `GET /rest-endpoints?withTests=true` adding `testSites` and `disabledTestSites` per
endpoint would answer it in one call. Modest scope, and it is the question that decides what to work
on next in a reengineering push.
- [ ] **148. `upms` reports 6315 examined / 6311 persisted / 0 failures — the four are unaccounted for**
```
GET /api/projects/upms → ingest: { filesExamined: 6315, filesPersisted: 6311,
filesFailed: 0, failures: [], incomplete: false }
```
Item 126 delivered exactly the metadata that makes this visible, and it immediately shows a gap it
does not explain: four files were walked, produced no persisted node, and are not failures. Probably
benign (an empty file, a file producing no module shell — cf. item 132), but "examined − persisted ≠
failed" is silently non-zero, and the consuming project's instructions still carry a rule saying a
404 from `upms` is no proof, with a filesystem cross-check as the documented workaround.
Either account for the difference (a `filesSkipped` with reasons) or state that examined-minus-
persisted is expected and why. Cheap, and it retires a standing distrust rule in a downstream
project.
- [ ] **149. Message numbers are findable per project but not across the pair**
Better than expected: `GET /upms/search/value?value=4080` returns the `##MSG-NR` assignment sites
(`DACOMEN0:40`, `DACTBEN0:40`, …), which is most of what a validator job needs. What is missing is
the other half — the same number in `pur` lives as a `LiteralId`/`ErrorField` enum constant, so
"where is 4080 raised on the Java side, and does it exist at all yet" is a second search in a second
project with a different result shape.
Falls out of item 143's cross-project work if the counterpart edge exists; not worth its own
mechanism otherwise. Filed so the connection is not lost.
- [ ] **150. `/modules/{name}/source` works well, and the guide steers agents away from it**
Probed and correct on both languages, including the encoding question — `upms` files are ISO-8859
and come back as proper UTF-8 (`…DB-Tabelle AGNT ändern…`), `?file=` reaches copycode that has no
module node (`ISICINDE.cpy:15-20`), Java line ranges are exact. Two small things:
* `agent-api-system-prompt.md:18` says to read files from disk and use the source endpoints
*"**only** when you have no filesystem access"*. For a consuming project whose own rules make the
graph the primary source and file reads the exception — and which enforces that with a hook on the
Java tree — that guidance is backwards. It is a documentation change, not a code change: say that
the endpoint is the appropriate path when the caller's policy prefers it, and note the ISO-8859
handling explicitly, since that is the reason an agent would otherwise reach for `iconv`.
* A copycode is reachable by path but not by name (`/upms/modules/ISICINDE/source` → `404
MODULE_NOT_FOUND`), while `viaCopycode`/`includePath` on other responses hand back exactly that
name. Accepting a copycode name would close the loop.
**Probed and dropped — do not re-file.** `409 AMBIGUOUS_NAME` already returns `candidates` **and**
`qualifiedNames` in `details` (verified on `pur/modules/AuthorizationInterceptor/context`) — items 115
and 125 covered it. Incremental refresh is item 129 and delivered (`?paths=`, `?changedOnly=`,
`ingest.incomplete`). Serving decoded source by line range is delivered, see item 150 above.