Skip to content

fix(semantics): restrict reflective attributes to owning metaclasses - #1835

Merged
HuiJun merged 1 commit into
developfrom
fix/reflective-metaclass-owned-attributes
Nov 2, 2026
Merged

HuiJun merged 1 commit into
developfrom
fix/reflective-metaclass-owned-attributes

Conversation

@devin-ai-integration

@devin-ai-integration devin-ai-integration Bot commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

What and why

Reflective reads now respect metaclass attribute ownership:

  • The implied ConjugatedPortDefinition answers only isAbstract and isSufficient in its Type-attribute branch, both false.
  • Definitions no longer answer Feature-owned isConstant.
  • isParallel is derived only for metaclasses conforming to StateDefinition or StateUsage.
  • Usage constancy stays on the existing reflectiveFeatureBoolean / FeatureIsConstant path.

Caller inspection confirmed that typed filters, queries and runtime reflection check metaclass/property applicability before reading; ok=false represents an absent or underived value, not a lost legitimate property. Parser/AST behavior is unchanged: an invalid constant part def prefix can populate Definition.IsConstant while producing a diagnostic.

Correct the existing unreleased conjugated-port entry and add a separate ownership-fix entry. No files from #1834 are changed.

Specification basis

KerML 8.3.3.1 defines Type attributes isAbstract and isSufficient; KerML 8.3.3.3.4 places isConstant on Feature. SysML v2 8.3.12.3 defines the owned conjugated port definition, while 8.3.18.5–8.3.18.6 place isParallel on StateDefinition and StateUsage. No compliance row or gate count changes.

How it was verified

  • Regression tests failed on the old implementation and pass with the fix. They cover absent attributes on ~Q, part/port definitions and part usages; true/false state parallelism; and unchanged Feature constancy.
  • go build ./..., go vet ./..., make lint, make changelog-check: pass. gofmt -l .: empty.
  • After ./scripts/download-pilot-corpora.sh and ./scripts/download-training-examples.sh, OPENSYSML_REQUIRE_PILOT_CORPORA=1 OPENSYSML_REQUIRE_TRAINING_CORPUS=1 go test -count=1 ./internal/semantic/... ./internal/translate/export/... ./tests/export/... ./internal/check/...: pass.
  • go -C tools test -count=1 ./referee/apijson/... ./oracle/baseline/...: pass, including the provenance guards; no re-recording needed.
  • After ./scripts/download-spec-examples.sh, OPENSYSML_REQUIRE_PILOT_CORPORA=1 OPENSYSML_REQUIRE_TRAINING_CORPUS=1 OPENSYSML_REQUIRE_SPEC_EXAMPLES=1 go test -count=1 ./tests/corpus -run TestCorpusRoundTrip: pass, not skipped. All 880 pinned verdicts remain unchanged: 835 stable, 7 whitespace-only, 38 refused; zero graph differences, unwritable files or unparseable files.
  • OPENSYSML_REQUIRE_PILOT_CORPORA=1 OPENSYSML_REQUIRE_TRAINING_CORPUS=1 OPENSYSML_REQUIRE_SPEC_EXAMPLES=1 go test -p 2 -timeout=30m ./...: pass. The initial default-timeout run timed out in cmd/sysml and tests/codegen; both pass in the lower-concurrency rerun (323s and 383s).
  • No baseline files, rows, source digests or generated counts changed.

Checklist

  • go test ./... and make lint pass locally (race suite runs in CI)
  • Tests added or updated for the change
  • Documentation extended where it already covers the surface (unreleased changelog corrected)
  • Changelog entry added as changes/unreleased/<slug>.<section>.md, not as an edit to CHANGELOG.md
  • Baselines and generated counts unchanged; no regeneration required
  • No internal work-item labels in the body, docs, or changelog

Co-Authored-By: jason.han <hanhuijun@gmail.com>
@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".

  • Disable automatic comment, CI, and merge conflict monitoring

@devin-ai-integration
devin-ai-integration Bot marked this pull request as ready for review November 2, 2026 01:44

@devin-ai-integration devin-ai-integration Bot left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔍 Devin Review: 1 flag

Not posted on this PR by your GitHub settings — view it in Devin Review. (Configure)

Devin Review

@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

The advisory flag about RDF/reflection attribute ownership is not a valid-source discrepancy. encoder.flags writes only true flags, and the parser sets IsParallel only for state definitions/usages.

Verified with the CLI built from this head: API-JSON omits isConstant and isParallel on part/port definitions and ~Q, includes true/false isParallel only on states, and preserves usage constancy. Turtle states only the two explicit parallel-state flags and the constant usage flag. constant part def Invalid; is rejected with “'constant' is a usage prefix” (exit 2), so the parser's retained invalid-definition bit does not enter conversion.

No exporter change is needed for this reflective-ownership fix.

@HuiJun
HuiJun merged commit 2e92371 into develop Nov 2, 2026
30 of 51 checks passed
@HuiJun
HuiJun deleted the fix/reflective-metaclass-owned-attributes branch November 2, 2026 03:00
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