refactor: collapse free annotation AST state#450
Open
hjotha wants to merge 5 commits intomendixlabs:mainfrom
Open
refactor: collapse free annotation AST state#450hjotha wants to merge 5 commits intomendixlabs:mainfrom
hjotha wants to merge 5 commits intomendixlabs:mainfrom
Conversation
Symptom: all open PRs against main failed the shared build-and-test job in make test-integration, even when their local build/test/lint validation passed. The failures reproduced on origin/main, so they were baseline CI instability rather than PR-specific regressions. Root cause: TestWatcherDebounce could allow stale timer callbacks to send an extra message under slow scheduling, nanoflow integration fixtures used MDL syntax that no longer matched the grammar, and the doctype mx-check harness did not classify known page/nanoflow showcase consistency errors as expected limitations. Fix: guard watcher debounce callbacks with a generation counter, tighten the watcher burst test, update nanoflow fixtures to current MDL syntax, and extend the known consistency-error allowlist for showcase-only limitations. Tests: make build Tests: go test ./cmd/mxcli/tui -run TestWatcherDebounce -count=20 -v Tests: ./bin/mxcli check mdl-examples/doctype-tests/02b-nanoflow-examples.mdl Tests: go test -tags integration -count=1 -timeout 30m ./mdl/executor -run 'TestRoundtripNanoflow_(Loop|EnumParameter|Annotations)|TestMxCheck_DoctypeScripts/02b-nanoflow-examples.mdl|TestMxCheck_DoctypeScripts/03-page-examples.mdl' -v Tests: make test Tests: make lint-go Tests: make test-integration
Symptom: ActivityAnnotations stored both FreeAnnotation and FreeAnnotations, creating a duplicated state invariant that tests and future code could accidentally violate. Root cause: the single-value field was kept as a convenience alias after multi-free-annotation support was added, so builder code needed a dedup helper solely to reconcile both fields. Fix: keep only FreeAnnotations, update visitor and builder paths to pass that slice directly, and add a bug-test script documenting the expected free-annotation roundtrip shape. Tests: ran make build, mxcli check for the new bug-test MDL file, focused visitor/executor free-annotation tests, make test, and make lint-go.
hjotha
force-pushed
the
submit/free-annotation-ast-single-field
branch
from
34cc42a to
9e3400a
Compare
AI Code ReviewReview SummaryThis PR refactors the free annotation handling in microflows by consolidating redundant state ( Critical IssuesNone Moderate IssuesNone Minor Issues
What Looks Good
RecommendationApprove with note: Please update PR description to include nanoflow example simplification and roundtrip test adjustments for transparent scope disclosure. Alternatively, split these unrelated improvements into a follow-up PR. The core free annotation refactor is ready for merge. Automated review via OpenRouter (Nemotron Super 120B) — workflow source |
Contributor
Author
|
CI stabilization note: merged #412 into this branch so it uses the same integration-CI baseline as the rest of the active PR set. |
Symptom: CI failed in TestMxCheck_DoctypeScripts/empty_java_action_argument.mdl because a Java action declared as RETURNS Void was written as an entity return type named .void, and Studio Pro reported CE1613. Root cause: the generic data-type visitor treats bare qualified names as entity/enumeration references. Java action return types reused that generic path, so the explicit Void spelling became a qualified name instead of ast.TypeVoid. Fix: add a Java-action return-type wrapper that maps unqualified Void to ast.TypeVoid while leaving generic data-type parsing unchanged for parameters and attributes. Tests: added visitor coverage for explicit RETURNS Void; verified mxcli check for the doctype fixture and the targeted integration subtest that failed in GitHub Actions.
AI Code ReviewCritical IssuesNone found. Moderate IssuesNone found. Minor IssuesNone found. What Looks Good
RecommendationApprove the PR. The changes represent a clean refactor that fixes a real bug in annotation handling while improving code maintainability. All tests pass, the bug test verifies the fix, and there are no syntax or architectural concerns. Automated review via OpenRouter (Nemotron Super 120B) — workflow source |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #449.
Summary
ActivityAnnotations.FreeAnnotationfield.FreeAnnotations []stringas the single source of truth in visitor and builder code.Validation
make build./bin/mxcli check mdl-examples/bug-tests/free-annotation-roundtrip.mdlgo test ./mdl/visitor ./mdl/executor -run 'FreeAnnotation|AnnotationsBeforePosition|AnnotationAfterPosition' -vmake testmake lint-go