fix(resolve): memoize an import's resolved target (#636) - #638
Merged
HuiJun merged 2 commits intoSep 28, 2026
Merged
Conversation
someshSandbox
added a commit
to someshSandbox/OpenSysML
that referenced
this pull request
Sep 27, 2026
A filter condition's own names resolve unfiltered (InCondition), and nothing resolved meanwhile may be memoized. importTargets kept such a target, so an ordinary lookup could reach a namespace the filter rejects. It is now neither read nor written while a condition resolves. TestAnImportTargetFoundWhileAConditionIsResolvedIsNotRemembered fails without this and passes with it (review finding on Open-MBEE#638). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This was referenced Sep 27, 2026
Analysing a view resolved each expose's target afresh on every name lookup in the view, and each resolution searched the other exposes' unresolved targets again, in every order: 11 exposes took 35 s and 12 did not finish. An import's resolved target is now kept in importTargets, hits only; a miss is still never memoized, since it may only mean that sibling imports were suspended. This matches the OMG pilot, which links an import's target once. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A filter condition's own names resolve unfiltered (InCondition), and nothing resolved meanwhile may be memoized. importTargets kept such a target, so an ordinary lookup could reach a namespace the filter rejects. It is now neither read nor written while a condition resolves. TestAnImportTargetFoundWhileAConditionIsResolvedIsNotRemembered fails without this and passes with it (review finding on Open-MBEE#638). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
someshSandbox
force-pushed
the
fix/636-memoize-import-targets
branch
from
September 28, 2026 02:37
780228a to
7bb778c
Compare
6 tasks done
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
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.
Fixes #636.
A view with many exposes and a metadata usage stalls validation:
Cause: each expose's target was resolved again on every lookup in the view, never memoized (it resolves under
inAllVisible), so every lookup searched the other exposes in every order.Change: a new
importTargetstable keeps each import's resolved target.InCondition(review finding).implicitParamsand counted inMemoSize.This matches the OMG pilot, where an import's target is a linked cross-reference resolved once.
Tests:
tests/model/expose_scaling_test.go: 20 exposes must analyse within 10 s. Fails ondevelop, passes here (0.5 s).filter_test.go,TestAnImportTargetFoundWhileAConditionIsResolvedIsNotRemembered: a target found in condition mode doesn't leak to an ordinary lookup.internal/semantic/...,tests/model/...andinternal/workspace/...all pass ondevelopwith this change.gofmtis clean.make lintwasn't run (tools not installed).Does not fix #633 (a
public importcycle, a different path).🤖 Generated with Claude Code