From 0b7a6fa05e6151e690322bf01a46539d92954f73 Mon Sep 17 00:00:00 2001 From: Nedelcho Delchev Date: Mon, 5 Oct 2026 11:59:14 +0300 Subject: [PATCH] The .test manifest carries the picker rule, the list columns and the parent state a child refuses The app-test manifest described a relation as "pick any row of the target" and a list as "a header per non-major:false field", so the generic flows built samples the generated app then rejected: - a picker narrowed by `pickable:` never offered the first target row, so the record never saved (#7663); - a list curated with `list:` was asserted to render a column the page deliberately hides - and only on the first instance that held a row, since an empty list never reaches the assertion (#7664); - a child guarded by a `forbidWhen` over its parent's status had its REST create answered 400 by a rule nothing in the manifest mentioned (#7667). Three keys close it, each in the manifest's own vocabulary rather than a second one: - a relation's `pickable` - `{when: [{by, op, value}], hide}`, parsed by `PickableSupport` from the same terms the generated picker evaluates; - a relation's `forbiddenTarget` - the entity's own `forbidWhen` terms that reach one hop through THAT relation, with the check's message, which is what the REST flow needs since it never sees a picker; - the entity's `list` - the `.model`'s `listOrder`, in order. All three are emitted only when authored, so an uncurated module's manifest is byte-identical. Fixes #7663 Fixes #7664 Fixes #7667 Co-Authored-By: Claude Opus 5 (1M context) --- components/engine/engine-intent/CLAUDE.md | 1 + .../apptest/AppTestIntentGenerator.java | 105 ++++++++++++++++++ .../apptest/AppTestIntentGeneratorTest.java | 96 ++++++++++++++++ 3 files changed, 202 insertions(+) diff --git a/components/engine/engine-intent/CLAUDE.md b/components/engine/engine-intent/CLAUDE.md index 055e696ff02..9a85cfb38f6 100644 --- a/components/engine/engine-intent/CLAUDE.md +++ b/components/engine/engine-intent/CLAUDE.md @@ -72,6 +72,7 @@ The dream is "no code, no modelling - just prompt": user describes what they wan - **Safe YAML loading is non-negotiable.** `IntentParser` constructs SnakeYAML with `SafeConstructor`, which blocks `!!type` / `!!new` tags. Intents arrive from LLM output and human paste; YAML deserialisation must never become a code-execution surface. Do not swap to `Constructor` for "ergonomics". - **One `.intent` file per project**, at the project root. There is no plan to support multiple intents per project - the whole model lives in one place so the LLM has the whole picture to diff against. (Re-evaluate if intents grow past ~2000 lines in practice; until then, one file.) - **In an intent project, model-layer files at the project root are owned by the regeneration pass; `/gen/` stays the template engine's.** Developers must not hand-edit the generated `.edm`/`.bpmn`/`.form`/... - anything hand-edited is overwritten, and files no longer backed by the intent are scrubbed on the next regeneration (so adding `app.intent` to a classic project hands ownership of its root-level model files to the intent engine - migrate them into the intent first). `gen/` keeps its existing platform meaning: the model-to-code templates' output folder, wiped by them on every regeneration. **Two artifacts are the deliberate exception - written ONCE and developer-owned afterwards: the `.print` document template and the `.test` app-test manifest.** Both are scaffolds a human is expected to adapt: `writeModelFileIfAbsent` / `keepExistingModelFile` create them only when absent and claim them either way, so the scrub (which owns both extensions) keeps them. For the `.test` this is load-bearing rather than a convenience - a behavioural test re-derived from the intent by the same toolchain that generates the code inherits the generator's blind spots and passes precisely when the generator is consistently wrong, so it is worth something only when a human can state independently what the module must do (#6755). If a refresh is ever wanted it must be an explicit action, never the default of every Generate. +- **The `.test` manifest states what the generated app REFUSES, not only what it offers (#7663 / #7664 / #7667).** `AppTestIntentGenerator` described a relation as "pick any row of the target" and a list as "a header per non-`major: false` field", so the generic flows of the app-test SDK built samples the app then rejected: a picker narrowed by `pickable:` never offered the first target row (the record never saved), a list curated with `list:` was asserted to render a column it deliberately hides (and only on the first instance that held a row, since an empty list passes), and a child guarded by a `forbidWhen` over its parent's status had its REST create answered 400 by a rule nothing in the manifest mentioned. Three keys close it, each carrying the manifest's own vocabulary rather than a second one: a relation's **`pickable`** (`{when: [{by, op, value}], hide}`, from `PickableSupport.parse` - the same terms the generated picker evaluates), a relation's **`forbiddenTarget`** (the entity's own `forbidWhen` terms that reach one hop through THAT relation, with the check's message - the REST half, which never sees a picker), and the entity's **`list`** (the `.model`'s `listOrder`, in order). All three are emitted only when authored, so an uncurated module's manifest is byte-identical. Covered by `AppTestIntentGeneratorTest`. - **Existing projects without an intent stay "classic"** (hand-edit EDM/BPMN/form as before). An "intent project" is detected by the presence of `app.intent` at project root. A future `reverse-engineer intent` command can scan EDM/BPMN/form and propose an intent file to migrate; out of scope for now. - **Mermaid renders the intent for visualisation**, read-only. We do NOT build a Mermaid round-trip editor (it is a poor authoring surface). Editing is via the LLM prompt + structured panel; the existing modelers are NOT re-used for intent projects (they would let developers edit gen/ in disguise). - **Run-once-fix-it via Claude.** When something can't be expressed, the answer is to extend `.intent` (add a field to the schema, add a generator that consumes it), not to leak into gen/. diff --git a/components/engine/engine-intent/src/main/java/org/eclipse/dirigible/components/intent/generator/apptest/AppTestIntentGenerator.java b/components/engine/engine-intent/src/main/java/org/eclipse/dirigible/components/intent/generator/apptest/AppTestIntentGenerator.java index 96cd8a09178..8f7ee843238 100644 --- a/components/engine/engine-intent/src/main/java/org/eclipse/dirigible/components/intent/generator/apptest/AppTestIntentGenerator.java +++ b/components/engine/engine-intent/src/main/java/org/eclipse/dirigible/components/intent/generator/apptest/AppTestIntentGenerator.java @@ -19,6 +19,8 @@ import org.eclipse.dirigible.components.intent.generator.IntentGenerationContext; import org.eclipse.dirigible.components.intent.generator.IntentNaming; import org.eclipse.dirigible.components.intent.generator.IntentTargetGenerator; +import org.eclipse.dirigible.components.intent.generator.CheckSupport; +import org.eclipse.dirigible.components.intent.generator.PickableSupport; import org.eclipse.dirigible.components.intent.generator.edm.CrossModelSupport; import org.eclipse.dirigible.components.intent.model.CheckIntent; import org.eclipse.dirigible.components.intent.model.EntityIntent; @@ -171,6 +173,13 @@ private static Map entityManifest(EntityIntent entity, Map listColumns = csv(string(edm.get("listOrder"))); + if (!listColumns.isEmpty()) { + out.put("list", listColumns); + } out.put("navGroup", string(edm.get("perspectiveNavId"))); out.put("api", "/" + sanitizeJavaIdentifier(string(edm.get("perspectiveName"))) + "/" + name + "Controller"); out.put("table", string(edm.get("dataName"))); @@ -383,6 +392,66 @@ private static List> fields(EntityIntent entity, Map. ==|!= } and refuses the write while it + * holds, so a runner building the target must land OUTSIDE every one of them. + * + * @param entity the record the checks are declared on + * @param relation the to-one being described + * @return the forbidding conditions, in authored order, each with the check's own message + */ + private static List> forbiddenTarget(EntityIntent entity, RelationIntent relation) { + List> forbidden = new ArrayList<>(); + for (CheckIntent check : entity.getChecks() == null ? List.of() : entity.getChecks()) { + if (!"forbidWhen".equals(check.getKind())) { + continue; + } + for (String authored : CheckSupport.terms(check.getWhen())) { + CheckSupport.Comparison comparison = CheckSupport.parse(authored); + int dot = comparison == null ? -1 + : comparison.property() + .indexOf('.'); + if (dot <= 0 || !comparison.property() + .substring(0, dot) + .equalsIgnoreCase(relation.getName())) { + continue; // a term about this record's own fields, or about another relation + } + Map condition = new LinkedHashMap<>(); + condition.put("by", IntentNaming.pascalCase(comparison.property() + .substring(dot + 1))); + condition.put("op", comparison.equal() ? "eq" : "ne"); + condition.put("value", literal(CheckSupport.unquote(comparison.literal()))); + if (check.getMessage() != null && !check.getMessage() + .isBlank()) { + condition.put("message", check.getMessage()); + } + forbidden.add(condition); + } + } + return forbidden; + } + + /** A manifest literal: a number stays a number, so the runner compares it with the stored id. */ + private static Object literal(String authored) { + try { + return Long.valueOf(authored.trim()); + } catch (NumberFormatException notANumber) { + return authored; + } + } + + /** The comma-separated attribute as a list, empty when the attribute is absent. */ + private static List csv(String value) { + List parts = new ArrayList<>(); + for (String part : value == null ? new String[0] : value.split(",")) { + if (!part.isBlank()) { + parts.add(part.trim()); + } + } + return parts; + } + private static List> relations(EntityIntent entity, IntentModel model, IntentGenerationContext context, Map> edmEntities) { Map usesByAlias = new LinkedHashMap<>(); @@ -442,6 +511,42 @@ private static List> relations(EntityIntent entity, IntentMo where.put("value", condition.getValue()); out.put("where", where); } + // pickable: (#7496) - a picker rule over the TARGET's rows. Without it the runner samples + // the first target row, the picker never offers it, and the record never saves (#7663). + // The terms carry the manifest's own vocabulary (`by`, as `where` does), plus whether a + // failing row is hidden outright or listed disabled. + if (relation.getPickable() != null) { + Map pickable = new LinkedHashMap<>(); + List> terms = new ArrayList<>(); + for (String authored : CheckSupport.terms(relation.getPickable() + .getWhen())) { + PickableSupport.Term term = PickableSupport.parse(authored); + if (term == null) { + continue; // the parser refused it; nothing to describe + } + Map condition = new LinkedHashMap<>(); + condition.put("by", IntentNaming.pascalCase(term.property())); + condition.put("op", term.op()); + if (!term.presence()) { + condition.put("value", literal(CheckSupport.unquote(term.literal()))); + } + terms.add(condition); + } + if (!terms.isEmpty()) { + pickable.put("when", terms); + pickable.put("hide", PickableSupport.hides(relation.getPickable())); + out.put("pickable", pickable); + } + } + // A state the TARGET must not be in for this record to be accepted at all (#7667): the + // child's own `forbidWhen` reaching one hop through this relation - "a credit note may + // only correct an issued invoice". The picker rule above is the UI half of the same thing + // and is often absent; this one is what the REST flow needs, since the create is refused + // with 400 whatever the form offered. + List> forbidden = forbiddenTarget(entity, relation); + if (!forbidden.isEmpty()) { + out.put("forbiddenTarget", forbidden); + } if (relation.isCrossModel()) { UsesIntent uses = usesByAlias.get(relation.getModel()); if (uses == null) { diff --git a/components/engine/engine-intent/src/test/java/org/eclipse/dirigible/components/intent/generator/apptest/AppTestIntentGeneratorTest.java b/components/engine/engine-intent/src/test/java/org/eclipse/dirigible/components/intent/generator/apptest/AppTestIntentGeneratorTest.java index 29e138b9189..2e696db1cfb 100644 --- a/components/engine/engine-intent/src/test/java/org/eclipse/dirigible/components/intent/generator/apptest/AppTestIntentGeneratorTest.java +++ b/components/engine/engine-intent/src/test/java/org/eclipse/dirigible/components/intent/generator/apptest/AppTestIntentGeneratorTest.java @@ -500,6 +500,102 @@ void marksTheRelationsTheFormRendersReadOnly() { .get("readOnly")); } + /** + * The three things a curated, guarded module needs the runner to know (#7663, #7664, #7667): which + * target rows the picker actually offers, which columns the list actually renders, and which parent + * state the child refuses outright. + */ + @SuppressWarnings("unchecked") + @Test + void carriesThePickerRuleTheListColumnsAndTheParentStateAChildRefuses() { + String intent = """ + name: billing + entities: + - name: InvoiceStatus + kind: setting + fields: + - { name: id, type: integer, primaryKey: true, generated: true } + - { name: name, type: string } + - name: Invoice + fields: + - { name: id, type: integer, primaryKey: true, generated: true } + - { name: number, type: string, length: 100 } + relations: + - { name: Status, kind: manyToOne, to: InvoiceStatus, function: EntityStatus, init: 1 } + - name: CreditNote + list: [number, date, Invoice] + checks: + - { kind: forbidWhen, when: "Invoice.Status == DRAFT", message: "A credit note can only correct an issued invoice" } + fields: + - { name: id, type: integer, primaryKey: true, generated: true } + - { name: number, type: string, length: 100 } + - { name: date, type: date } + - { name: taxEventDate, type: date } + relations: + - name: Invoice + kind: manyToOne + to: Invoice + required: true + pickable: { when: [Status != 1], message: "Only an issued invoice can be corrected" } + seeds: + - name: invoice-statuses + entity: InvoiceStatus + rows: + - { id: 1, name: DRAFT } + - { id: 2, name: ISSUED } + """; + Map> edmEntities = new LinkedHashMap<>(); + edmEntities.put("InvoiceStatus", edmEntity("InvoiceStatus", "Invoice Status", "Invoice Statuses", "MANAGE_LIST", "Settings", + "billing", "KF_MOD_BILLING_INVOICESTATUS", false)); + edmEntities.put("Invoice", + edmEntity("Invoice", "Invoice", "Invoices", "MANAGE_LIST", "Billing", "billing", "KF_MOD_BILLING_INVOICE", false)); + Map creditNote = edmEntity("CreditNote", "Credit Note", "Credit Notes", "MANAGE_LIST", "Billing", "billing", + "KF_MOD_BILLING_CREDITNOTE", false); + // what applyListColumns writes onto the .model entity for a declared `list:` + creditNote.put("listOrder", "Number,Date,Invoice"); + edmEntities.put("CreditNote", creditNote); + + Map manifest = AppTestIntentGenerator.buildManifest("billing", "billing", IntentParser.parse(intent), edmEntities); + Map note = entity(manifest, "CreditNote"); + + // #7664: the curated columns, in order - the list renders these and no other, whatever each + // field's own `major` says, so TaxEventDate must not be expected as a header. + assertEquals(List.of("Number", "Date", "Invoice"), note.get("list")); + + Map invoice = ((List>) note.get("relations")).get(0); + + // #7663: the picker offers only the rows the rule admits, so the runner must sample one of + // those - the seeded name is already an id by the time the manifest is written. + Map pickable = (Map) invoice.get("pickable"); + assertEquals(List.of(Map.of("by", "Status", "op", "ne", "value", 1L)), pickable.get("when")); + assertEquals(Boolean.FALSE, pickable.get("hide")); + + // #7667: and the REST flow, which never sees a picker, is told the same thing by the check + // that would refuse its create with 400. + List> forbidden = (List>) invoice.get("forbiddenTarget"); + assertEquals(1, forbidden.size()); + assertEquals("Status", forbidden.get(0) + .get("by")); + assertEquals("eq", forbidden.get(0) + .get("op")); + assertEquals(1L, forbidden.get(0) + .get("value")); + assertEquals("A credit note can only correct an issued invoice", forbidden.get(0) + .get("message")); + } + + /** An uncurated, unguarded relation carries none of the three - the manifest stays as it was. */ + @SuppressWarnings("unchecked") + @Test + void anUncuratedEntityCarriesNeitherListNorPickerRule() { + Map city = entity(AppTestIntentGenerator.buildManifest("countries", "countries", model, edm()), "City"); + + assertNull(city.get("list")); + Map country = ((List>) city.get("relations")).get(0); + assertNull(country.get("pickable")); + assertNull(country.get("forbiddenTarget")); + } + // ---- helpers: a minimal .model-shaped metadata map ------------------------------------------- private static Map> edm() {