Skip to content

Generic alter <type> X { set / insert / replace / drop }: grammar, target resolver, page family (#712, part 1) - #722

Merged
ako merged 8 commits into
mainfrom
feature/712-generic-alter
Sep 26, 2026
Merged

ako merged 8 commits into
mainfrom
feature/712-generic-alter

Conversation

@ako

@ako ako commented Sep 26, 2026 •

Copy link
Copy Markdown
Owner

Part of #712 (partial; the issue stays open for the remainder)

Plan item 4.1 of PROPOSAL_mdl_beta_syntax_freeze.md §8.3 / §9, implementing ADR-0012 decision 2. First PR of two: the generic grammar, the per-document-type target resolver interface, and the page family (alter page / alter snippet / alter layout) ported onto it. alter workflow is the follow-up (see below).

What changes

One grammar rule. alter page|snippet|layout Module.Name { … } now all parse through

ALTER alterDocumentType qualifiedName LBRACE alterOperation+ RBRACE

with the shared operations alterSet / alterInsert / alterReplace / alterDrop, the shared address alterTarget (name, name.member, 'caption', each with optional @n) and alterFragment (the create body of the document type — pageBodyV3 today). The three per-type ALTER PAGE/LAYOUT/SNIPPET … alterPageOperation+ alternatives and widgetRef are gone; every existing alter page/snippet/layout test now runs through the generic rule, unchanged.

Canonical form:

alter page Shop.Order_Edit {
  set (Caption: 'Save', ButtonStyle: Success) on btnSave;
  set (Title: 'Edit order');                -- the document itself
  insert after txtName { textbox txtMiddle (Label: 'Middle', Attribute: Middle) }
  replace footer1 with { footer f1 { … } }
  drop txtOld, dgOrders.Total;
};

Target resolver interface — backend.AlterTargetResolver (mdl/backend/alter_target.go): ResolveAlterTarget(AlterTarget) (AlterTargetMatch, error), plus AlterTargetError (a miss, or an ambiguity listing the matches as @1 … @n — never a guess) and PickAlterTargetMatch, the shared @n semantics the microflow resolver (#713) can reuse. PageMutator embeds it, so both backends implement it:

  • modelsdk (pagemutator.Mutator): widget by name, grid.Column, scroll-container region layoutContainer.top, using the same finders the operations use, so resolution cannot accept what an operation then misses, and a miss reads as before (widget "x" not found, the column list, the region refusals).
  • --mcp: widget by name; a dotted target gets an explicit "not yet supported by the MCP backend" (no MCP page operation supports columns/regions today).

The executor resolves every operation's targets before running it (resolveAlterPageTargets); a refusal stops the statement before the operation and before Save.

Old forms are aliases, parse to the identical operation, and warn from check and exec:

Old Canonical Code
set K = v [on w], set (K = v, …) [on w] set (K: v, …) [on w] MDL-DEPR101
set K: v [on w] (no parentheses) set (K: v) [on w] MDL-DEPR102
drop widget a, b drop a, b MDL-DEPR103

check also refuses, with no project, a caption / @n / 3-part target on a page (MDL-ALTER01) — the grammar accepts every address form some document type needs; the rule is the page resolver's own backend.CheckPageAlterTarget, not a copy.

Decisions the ADRs did not settle

  1. Deprecation registry (Deprecation registry for MDL aliases (MDL-DEPRnnn), generalising MDL065 #709) is not merged, so the aliases live in a small table (mdl/executor/alter_aliases.go) with exactly Deprecation registry for MDL aliases (MDL-DEPRnnn), generalising MDL065 #709's entry shape {code, old, canonical, rewrite, removed in}, and the grammar marks each alias alternative // alias: MDL-DEPRnnn. TestAlterAliasGrammarMarkersMatchTable pins marker ↔ table both ways — the check Deprecation registry for MDL aliases (MDL-DEPRnnn), generalising MDL065 #709 generalises. Codes are provisional, numbered from 101 to stay clear of Deprecation registry for MDL aliases (MDL-DEPRnnn), generalising MDL065 #709's seed entries; Deprecation registry for MDL aliases (MDL-DEPRnnn), generalising MDL065 #709 should absorb or renumber them.
  2. Page-family operations with no generic spelling yet stay as they are, not aliases: set layout = X [map (…)], drop template for E in lv, add variables, drop variables. (A set (Layout: …) spelling needs a home for map; left for the R2/R3 alias work.)
  3. Page targets are names only. A caption or @n on a page is refused rather than ignored: widget names are unique within a page, so neither has anything to mean (ADR-0010 R11 strictness).
  4. The AST keeps AlterPageStmt / WidgetRef (now with Caption / Ordinal) because every page validator and ~25 test files speak them; the generic-ness lives in the grammar and the resolver.
  5. Syntax help, the alter-page skill, the quick reference and the two docs-site alter pages now teach the canonical form. Other skills still contain the old spellings — they work and warn; rewriting them is fmt --upgrade's job (Phase 3).

Follow-ups (not in this PR)

  • Port alter workflow (wfmutator) onto the generic grammar: alterFragment gains workflowBody, the workflow mutator implements AlterTargetResolver (activity by name or caption, @n), and the current set activity … / insert outcome … on … forms become registered aliases.
  • Register MDL-DEPR101..103 in the Deprecation registry for MDL aliases (MDL-DEPRnnn), generalising MDL065 #709 registry (and fmt --upgrade rewrites) once it lands.
  • Rewrite remaining old alter page spellings in other skills/docs.

Test plan (what I actually ran)

  • New tests: mdl/visitor/visitor_alter_generic_test.go (canonical set/drop, target forms incl. 'caption'@2 and layoutContainer.top, aliases flagged, doc-specific ops, @0 refused), mdl/backend/alter_target_test.go, mdl/backend/pagemutator/alter_target_test.go, mdl/backend/mcp/page_mutator_alter_target_test.go, mdl/executor/cmd_alter_generic_test.go (every target goes through the resolver; a refusal stops before the op and Save), mdl/executor/alter_aliases_test.go (warnings with a canonical control that must not warn; grammar-marker ↔ table; MDL-ALTER01 with a name-target control).
  • Revert checks — each fix stubbed, test watched failing, restored:
    • resolver call removed from execAlterPage → resolved targets: got [], want [a btnSave ctn dg.Total hdr lv] and want the resolver's AlterTargetError, got <nil>;
    • alias validation unhooked → codes: got [], want [MDL-DEPR101 MDL-DEPR101 MDL-DEPR102 MDL-DEPR103];
    • page address check removed from the resolver → caption: want a by-name refusal, got widget "" not found;
    • MDL-ALTER01 unhooked → want 2 MDL-ALTER01 errors, got [];
    • grammar: the pre-change binary rejects the canonical script (no viable alternative at input 'Caption:') and accepts the old one; the new binary accepts both and warns MDL-DEPR101 on the old one.
  • End to end on a copy of testdata/expr-checker/minimal.mpr (Mendix 11.6.6): canonical set (Caption: …) on actionButton11, insert after, drop → Altered page, describe shows the change; drop 'Learn more' → refused by the resolver; drop nosuchwidget → widget "nosuchwidget" not found. No new BSON is written by this change (the operations are the existing ones), so no Studio Pro / mx check run was needed for it.
  • make build: pass. make lint: pass. make test: every package passes except mdl/backend/modelsdk, which hit the default 10-minute go test timeout under the shared machine's load (fixture copies; untouched by this change) — re-run with -timeout 60m together with mdl/executor, mdl/visitor and cmd/mxcli/...: all pass (modelsdk 670s). /tmp was full from other sessions, so tests ran with TMPDIR on the workspace disk.

🤖 Generated with Claude Code

ako and others added 8 commits September 26, 2026 20:50
…or the page family (#712)

One grammar rule, ALTER alterDocumentType qualifiedName { alterOperation+ },
replaces the separate ALTER PAGE / SNIPPET / LAYOUT alternatives, with a
shared target address (name, name.member, 'caption', @n) and fragment.

Targets resolve through a per-document-type backend.AlterTargetResolver,
which PageMutator now embeds: the modelsdk page mutator resolves widgets,
grid columns and scroll-container regions with the finders the operations
use; the MCP mutator resolves widgets and refuses dotted targets
explicitly. The executor resolves every operation's targets before
running it.

The old spellings (set K = v, set K: v without parentheses, drop widget)
parse to the identical operation and warn as MDL-DEPR101..103, from a table
shaped like the #709 registry and pinned to `// alias:` grammar markers. A
caption or @n target on a page is refused at check time (MDL-ALTER01).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…layouts (#712)

mxcli syntax page/snippet/layout.alter, the alter-page skill, the quick
reference and the docs-site alter-page pages now show set (Key: value) and
drop <target>, and name the MDL-DEPR101..103 aliases.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ADR-0011 requires an alias to parse to exactly what its canonical form
parses to. Twelve pairs (set with =, unparenthesised set, page-level set,
quoted key, action, named action slot, datasource, visible, expression,
column target, drop widget) are compared with reflect.DeepEqual after
clearing the Legacy marker. Control: stubbing the visitor to drop a target
on the alias path fails the column_target and drop_widget cases.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…an ambiguous column (#712)

The resolver ran columnMatchCount for every bare name, so a real widget
whose name is also a derived column name in two grids was refused as an
ambiguous column, although set had always gone to the widget
(SetWidgetProperty raises the ambiguity only when the name resolved to a
column). That is a new rejection, which ADR-0011 allows only behind the
language header. The ambiguity is now checked only when the name resolved
to a column; drop/replace/insert keep their own check.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… form (#712)

Two refusals told the user to write `set Action = ... on w`, which now
warns MDL-DEPR101.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…e script (#712)

AP14 exercises set (K: v) at page level and on a widget, insert, replace
and drop without the widget keyword on a page and a snippet, so the
canonical path is covered by the doctype mx-check run, next to the old
spellings above it (which still run and warn).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ey: value) form (#712)

The skill's property table and column examples still taught
`set Key = value on w`, which now warns MDL-DEPR101, so an agent following
the skill produced a deprecation warning on every alter. All 29 set
examples in the skill now parse through check without a warning.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…...) (#712)

Follows the hint change: the refusal on a built-in widget now names
`set (Action: …)`.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@ako

ako commented Sep 26, 2026

Copy link
Copy Markdown
Owner Author

Independent review: findings and fixes pushed

What I verified by running it

  • make build, make lint and make test pass. The only failure in make test was an assertion that my own hint change broke; it is fixed in 2e4d9be and the package passes again.
  • Revert checks. I removed the resolver call from execAlterPage and unhooked validateAlterAliases and validateAlterPageAddresses. All four new executor tests then failed with the symptoms the PR description gives.
  • Old scripts build the same result. I ran the unmodified 33-alter-page-examples.mdl on copies of testdata/expr-checker/minimal.mpr, once with an origin/main binary and once with the PR binary. describe output of Product_Overview, Product_NewEdit and Product_Card_Snippet is identical. layouts.mdl (the alter layout region insert and drop widget) also runs cleanly.
  • mx check. mx check (11.6.6) reports CE0535 ("column weights add up to -1") on that script. The origin/main binary produces the same error, so it is not caused by this PR.
  • Drops are resolved before anything changes. drop text48, nosuch stops with widget "nosuch" not found, and text48 is still on the page.

Fixes pushed on this branch (tests first)

  1. 106e453: a test that each old spelling builds the identical operation as the canonical form. It compares the AST with reflect.DeepEqual over 12 pairs, as ADR-0011 requires. For a control, I stubbed the visitor to drop a target on the alias path, and the test failed.
  2. c19d704: a new rejection. The page resolver ran the column-ambiguity check on every bare name. So set (Class: …) on Merchant, where Merchant is a real widget and also a derived column name in two grids, was refused as "column is ambiguous". The operation itself (SetWidgetProperty) has always accepted this. The ambiguity check now runs only when the name resolved to a column. The test failed before the fix with that exact message.
  3. 4028134 and 2e4d9be: two refusal hints told users to write set Action = … on w, which now triggers the deprecation warning. They now name set (Action: …).
  4. e7e88c5: the alter-page skill still taught set Key = value in its whole property table and in the column examples. All 29 examples are now in the canonical form and pass check with no warnings.
  5. 3cb20a7: added doctype example AP14, which covers the canonical form on a page and a snippet: page-level set, set on a widget, insert, replace, and drop without the widget keyword. It exec'd cleanly and describe confirms the changes.

Needs the maintainer's judgement

  • MDL-DEPR102 (set Key: value without parentheses) never parsed on main. The old grammar only accepted =. So this PR adds a new spelling and deprecates it in the same change. It helps LLM-written scripts, but the registry will list as deprecated a form that never shipped. The PR description calls it an "old spelling", which is not accurate.
  • The deprecation warnings give only the document (at M.P), with no line number. A script with 12 warnings is hard to fix by hand.
  • Pre-existing and unrelated: the skill's "Limitations" item 1 says set Action = … on btnSave is a parse error. It is not, since Action can be set. I did not change it.

@ako
ako merged commit 861d535 into main Sep 26, 2026
17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant