Skip to content

Commit bccc67c

Browse files
akoclaude
ako
and
claude
committed
docs: mark Changes 1, 2, and 3 as implemented in agentic architecture proposal
Changes 1 (gitignore generated parser), 2 (split grammar by domain), and 3 (command self-registration via explicit NewRegistry()) are all shipped. Updated each section to reflect actual implementation details and replaced the forward- looking summary table with done/remaining status. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
1 parent 5cf8513 commit bccc67c

1 file changed

Lines changed: 45 additions & 85 deletions

File tree

‎docs/11-proposals/PROPOSAL_agentic_architecture_improvements.md‎

Lines changed: 45 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -32,117 +32,77 @@ The problems are:
3232

3333
## Proposed Changes
3434

35-
### Change 1: Remove Generated Parser Files from Git
35+
### Change 1: Remove Generated Parser Files from Git ✅ Implemented
3636

37-
**What:** Add `mdl/grammar/parser/*.go` to `.gitignore` and stop tracking them. The `.g4` source files remain in git. Generated files are produced locally and in CI by running `make grammar`.
37+
**Status:** Shipped. `mdl/grammar/parser/` is excluded via `.gitignore` (line 42–43). The directory is not tracked; contributors regenerate with `make grammar`.
3838

39-
**Why this eliminates the conflict:** The 119,737-line generated file is the source of every grammar-related merge conflict. The file is deterministically derived from the `.g4` source — there is no information in it that isn't in the grammar. Removing it from git means two branches that both add syntax no longer produce a file-level conflict; only the human-readable `.g4` source can conflict.
40-
41-
**Prerequisites:** Contributors already have ANTLR4 installed (this has been verified — all grammar contributors have been able to run `make grammar` without issue). The barrier that would normally exist (requiring Java for ANTLR4) is not a practical concern here.
42-
43-
**CI impact:** The CI pipeline must install ANTLR4 and run `make grammar` before `make build` or `make test`. The `push-test.yml` and `release.yml` workflows currently have no grammar step — both need a setup step added.
44-
45-
**Implementation steps:**
46-
1. Add to `.gitignore`:
47-
```
48-
# Generated ANTLR4 parser (regenerate with: make grammar)
49-
mdl/grammar/parser/*.go
50-
mdl/grammar/parser/*.interp
51-
mdl/grammar/parser/*.tokens
52-
```
53-
2. Run `git rm --cached mdl/grammar/parser/*.go mdl/grammar/parser/*.interp mdl/grammar/parser/*.tokens`
54-
3. Add to CI workflows before the build step:
55-
```yaml
56-
- name: Install ANTLR4
57-
run: pip install antlr4-tools
58-
- name: Generate parser
59-
run: make grammar
60-
```
61-
4. Add `make grammar-check` target that regenerates and diffs — to catch cases where grammar source changed but `make grammar` was not run before committing
39+
**What was done:** Added `mdl/grammar/parser/` to `.gitignore`. The `.g4` source files remain in git; the generated Go parser is produced locally and in CI by `make grammar`. This eliminates the 119,737-line generated file as a source of merge conflicts entirely.
6240

6341
---
6442

65-
### Change 2: Split the Grammar Source by Domain
43+
### Change 2: Split the Grammar Source by Domain ✅ Implemented
6644

67-
**What:** Use ANTLR4's `import` directive to split `MDLParser.g4` into domain-specific grammar files, imported by a thin master grammar.
45+
**Status:** Shipped. `MDLParser.g4` is now a thin master grammar; domain rules live in `mdl/grammar/domains/`.
6846

69-
**Why this reduces the remaining conflict surface:** After Change 1, grammar conflicts can only occur on the `.g4` source files. A monolithic 4,082-line grammar means two PRs touching different domains (e.g. microflows and pages) still conflict at the source level. Splitting by domain means those PRs are fully independent.
70-
71-
**Proposed split:**
47+
**What was done:** The monolithic parser grammar was split into nine domain files imported via ANTLR4's `import` directive:
7248

7349
```
7450
mdl/grammar/
75-
MDLParser.g4 # master grammar: imports only, top-level statement rule
51+
MDLParser.g4 # master grammar: import directives + top-level statement rule
7652
MDLLexer.g4 # unchanged — tokens are shared across domains
7753
domains/
78-
MDLDomainModel.g4 # entity, attribute, association rules
79-
MDLMicroflow.g4 # microflow, nanoflow, activity rules
80-
MDLPage.g4 # page, snippet, widget rules
81-
MDLSecurity.g4 # roles, access rules, grant/revoke
82-
MDLNavigation.g4 # navigation profiles, menus
83-
MDLWorkflow.g4 # workflow, user task, decision rules
84-
MDLService.g4 # REST, OData, published services
85-
MDLCatalog.g4 # catalog queries, show/describe
86-
MDLSettings.g4 # project settings, constants, enumerations
87-
MDLSession.g4 # SET, USE, session-level commands
54+
MDLAgent.g4
55+
MDLCatalog.g4
56+
MDLDomainModel.g4
57+
MDLMicroflow.g4
58+
MDLPage.g4
59+
MDLSecurity.g4
60+
MDLService.g4
61+
MDLSettings.g4
62+
MDLWorkflow.g4
8863
```
8964

90-
The master `MDLParser.g4` becomes:
91-
```antlr
92-
parser grammar MDLParser;
93-
import MDLDomainModel, MDLMicroflow, MDLPage, MDLSecurity,
94-
MDLNavigation, MDLWorkflow, MDLService, MDLCatalog,
95-
MDLSettings, MDLSession;
96-
options { tokenVocab=MDLLexer; }
97-
statement : domainStatement | microflowStatement | pageStatement | ... ;
98-
```
65+
Two PRs touching different domains now edit independent files and cannot conflict at the grammar source level. The generated parser is still a single file — the split is source-level only; all visitor/listener code is unchanged.
9966

100-
**Note:** ANTLR4's `import` merges rules into the main grammar at generation time. The generated parser is still a single file — the split is a source-level convenience only. All existing listener/visitor code continues to work unchanged.
67+
---
10168

102-
**Implementation steps:**
103-
1. Identify rule boundaries in current `MDLParser.g4` by domain
104-
2. Extract each domain's rules into a `domains/MDL*.g4` file
105-
3. Replace extracted rules in `MDLParser.g4` with `import` directives
106-
4. Run `make grammar` and verify generated parser is functionally identical (test suite must pass)
107-
5. Update `mdl/grammar/Makefile` to list the domain files as dependencies
69+
### Change 3: Command Self-Registration in the Executor ✅ Implemented
10870

109-
---
71+
**Status:** Shipped. See `mdl/executor/registry.go` and `mdl/executor/register_stubs.go`.
11072

111-
### Change 3: Command Self-Registration in the Executor
73+
**What was proposed:** Replace the central dispatch switch with a registration pattern using `init()` so each `cmd_*.go` file self-registers its handler, making a new command self-contained in one file.
11274

113-
**What:** Replace the central dispatch switch in the executor with a registration pattern where each `cmd_*.go` file self-registers its handler on package init.
75+
**What was implemented:** The registry pattern without `init()`. `NewRegistry()` in `registry.go` is the single composition root that calls 29 named `registerXxxHandlers(r)` functions — one per domain. The `init()` approach was explicitly rejected because it creates package-level global state that breaks test isolation.
11476

115-
**Why this helps agentic development:** Currently, adding a new command requires: (a) creating a `cmd_*.go` file, (b) finding and editing the dispatch switch in `executor.go`, and (c) knowing the right context type. An agent has to read across multiple files to understand the pattern. With self-registration, a new command is entirely contained in its own file — no other file needs to change.
77+
**Why `init()` was not used:** With `init()`-based registration, every import of the `executor` package pre-populates a global handler map before any test runs. This makes it impossible to create a registry with zero handlers for targeted isolation tests. The existing test suite depends on `emptyRegistry()` (a factory that returns a handler-free `*Registry`) for six tests covering dispatch, completeness, and duplicate-registration panics. `init()` also makes duplicate-registration panics surface at package-load time rather than at `NewRegistry()`, producing an obscure failure with no clear test attribution.
11678

117-
**Proposed pattern:**
79+
**How it works now:**
11880

11981
```go
120-
// In each cmd_*.go file:
121-
func init() {
122-
executor.Register(ast.KindCreateEntity, handleCreateEntity)
123-
executor.Register(ast.KindAlterEntity, handleAlterEntity)
124-
}
125-
126-
func handleCreateEntity(ctx *executor.ExecContext, stmt ast.Statement) error {
127-
s := stmt.(*ast.CreateEntityStatement)
128-
return ctx.Backend.DomainModel().CreateEntity(s.Name, ...)
82+
// mdl/executor/registry.go — the composition root
83+
func NewRegistry() *Registry {
84+
r := &Registry{handlers: make(map[reflect.Type]StmtHandler)}
85+
// Registration functions are called here explicitly (no init()).
86+
registerEntityHandlers(r)
87+
registerMicroflowAndNanoflowHandlers(r)
88+
// ... 27 more domain-specific register calls
89+
return r
12990
}
13091
```
13192

13293
```go
133-
// In executor/registry.go:
134-
var handlers = map[ast.StatementKind]HandlerFunc{}
135-
136-
func Register(kind ast.StatementKind, fn HandlerFunc) {
137-
handlers[kind] = fn
94+
// mdl/executor/register_stubs.go — one function per domain
95+
func registerEntityHandlers(r *Registry) {
96+
r.Register(&ast.CreateEntityStmt{}, func(ctx *ExecContext, stmt ast.Statement) error {
97+
return execCreateEntity(ctx, stmt.(*ast.CreateEntityStmt))
98+
})
99+
// ...
138100
}
139101
```
140102

141-
**Implementation steps:**
142-
1. Add `executor/registry.go` with `Register()` and the handler map
143-
2. Replace the central dispatch switch with a lookup into the registry
144-
3. Migrate existing `cmd_*.go` files to register via `init()` — one file at a time, verifiable by running tests after each
145-
4. Remove the dispatch switch from `executor.go` once all commands are migrated
103+
**Adding a new command today:** Create `cmd_yourfeature.go` with the handler function, then add a `registerYourFeatureHandlers(r)` call in `NewRegistry()` and a corresponding stub function in `register_stubs.go`. The completeness test (`TestNewRegistry_Completeness`) will fail at CI if the registration step is missed.
104+
105+
**Agent discovery cost:** An agent can read `register_stubs.go` as the canonical index of all registered commands and their handler signatures. Any existing `registerXxxHandlers` function is a complete, copy-pasteable example.
146106

147107
---
148108

@@ -198,10 +158,10 @@ Because Go's `internal/` rule allows only the parent package and its children to
198158

199159
| Change | Problem solved | Risk | Effort |
200160
|---|---|---|---|
201-
| 1. Gitignore generated parser | Eliminates 119k-line conflict entirely | Low | Low |
202-
| 2. Split grammar by domain | Reduces source-level grammar conflicts | Medium (grammar refactor) | Medium |
203-
| 3. Command self-registration | Reduces agent discovery cost | Medium (executor refactor) | Medium |
161+
| 1. Gitignore generated parser | Eliminates 119k-line conflict entirely | ✅ Done | — |
162+
| 2. Split grammar by domain | Reduces source-level grammar conflicts | ✅ Done | — |
163+
| 3. Command self-registration | Reduces agent discovery cost | ✅ Done — explicit `NewRegistry()`, no `init()` | — |
204164
| 4. Code-generate mock | Eliminates mock drift, reduces sync errors | Low | Low |
205165
| 5. Compiler-enforced boundary | Converts checklist rule to compile error | Medium (needs design) | Medium |
206166

207-
**Recommended order:** Changes 1 and 4 first — both are low risk, high value, and immediately unblock parallel development. Changes 2 and 3 next, ideally in separate PRs. Change 5 needs a design decision on the `internal/` boundary approach before implementation.
167+
**Status:** Changes 1, 2, and 3 are shipped. Remaining work: Change 4 (code-generate mock) and Change 5 (compiler-enforced boundary, needs design decision on the `internal/` approach).

0 commit comments

Comments
 (0)