Skip to content

fix(parser): reject frame, filter and accept outside their bodies; declare extension libraries non-standard - #882

Open
devin-ai-integration[bot] wants to merge 9 commits into
developfrom
fix/view-body-frame-conformance
Open

devin-ai-integration[bot] wants to merge 9 commits into
developfrom
fix/view-body-frame-conformance

Conversation

@devin-ai-integration

@devin-ai-integration devin-ai-integration Bot commented Oct 4, 2026 •

Copy link
Copy Markdown
Contributor

What and why

The pinned OMG pilot validator (tag 2026-08) found two conformance gaps on develop.

1. Body members accepted in bodies whose grammar doesn't allow them. frame inside a view (examples/views-demo.sysml:176, examples/self-model/views.sysml:197) parsed cleanly, even under -strict. I compared OpenSysML and the pilot on every body-specific member across view / view def / viewpoint / viewpoint def / rendering / rendering def bodies (222 fixtures) and found three more gaps of the same kind. Each now follows the project's existing classification:

Member Grammar home Now
frame concern c : C; / frame c; RequirementBodyItem (requirement, concern, viewpoint) parser error elsewhere, via ownedMembers like subject/stakeholder/render
filter …; PackageBodyElement, view and view def bodies parser error elsewhere (new bodyPackage context for packages and the file root)
accept … action node ActionBodyItem (action, calc, case bodies) parser error elsewhere, like send/assign; state bodies are unchanged
one-name then t; ActionBodyItem, plus the state body's target transition nonstandard-notation outside action/state bodies (warning by default, error under -strict), extending the existing target-succession check; if g then t; / else t; there were already parser errors

Viewpoint frames (views-demo.sysml:135, self-model/views.sysml:9) are valid and stay clean.

Behaviour change in %view. Until now, conformance required the view to re-frame every concern of its viewpoint, and reported violated (framed by the viewpoint but not by the view) otherwise. Valid SysML can't write that, so the requirement is gone: each concern the satisfied viewpoint frames is evaluated directly against the elements the view exposes. ConcernConformance.FramedBy/FramedIn and the view-framing matching (own, inherited, nested, by name) are removed, and their tests are replaced with tests of the new rule. The examples drop the redundant view frame (the viewpoint already frames the same concern), as does disposal-robot-demo/robot.sysml. %view overview output is unchanged for views-demo and the self-model.

2. Extension libraries marked standard. DiagramLayout, IdentityMetadata, MigrationMetadata and SysMLValidation are non-normative OpenSysML extensions, but were declared standard library package. They are now library package. What this changes:

  • Element ids: no change. Normative UUIDs come from the library tier (Kernel/Systems/Domain), not from isStandard, and the OpenSysML Libraries tier never had them.
  • RDF export: these four packages no longer export sysml:isStandard true (false flags aren't written).
  • stdlib.snapshot: regenerated (make stdlib-snapshot).
  • Library-version recognition: a user copy is matched on its library/standard keywords, so a copy now has to be written library package to stand in for the bundled file.
  • Docs: pages that quote these declarations are updated.

Specification basis

  • SysML v2 pilot grammar SysML.xtext (tag 2026-08): ViewBodyItem, ViewDefinitionBodyItem, RequirementBodyItem (FramedConcernMember), PackageBodyElement (ElementFilterMember), ActionBodyItem (AcceptNode, TargetSuccessionMember).
  • KerML 1.0 LibraryPackage::isStandard: only for standard / normative model libraries.
  • docs/project/spec-compliance.md: the view-conformance row text is updated; its status is unchanged (⚠️ Approximate, tool-defined verdict rules).

How it was verified

Pilot results, before → after:

Input Before After
examples/self-model + OpenSysML Libraries 2 syntax errors (views.sysml:197), 4 "should not be marked as standard" warnings 0 errors (the 46 existing bound-feature/duplicate-name warnings in document.sysml are unchanged)
examples/views-demo.sysml + OpenSysML Libraries 2 syntax errors (:176), 4 warnings 0 errors, 0 warnings
OpenSysML Libraries alone 0 errors, 4 warnings 0 errors, 0 warnings
  • Fixture comparison: across all 222 body × member cases plus extra accept / if…then / else cases, every case the pilot rejects as a syntax error is now an error from us under -strict. The remaining mismatches are cases where both sides already report non-syntax errors.
  • Pilot rejection oracle: new cases g82–g85 (frame in view / view def bodies, filter outside a package or view, accept outside an action body) and x11 (target succession outside an action body). Baseline is now 317 cases: 308 rejected by both, 0 pilot-only, 9 ours-only.
  • Pilot differential: 345 → 347 of 384 files fully agreeing; pilot-only rows 1640 → 1634. views-demo.sysml and self-model/views.sysml now agree fully; robot.sysml keeps its one adjudicated second-objective row. The pilot-differential.md narrative and counts are updated.
  • Corpus gates: pilot-corpora, training-examples and corpus round-trip gates pass with no movement; nothing was re-baselined.
  • Full checks: go build ./..., go vet ./..., gofmt -l . (empty), go test ./..., go test -C tools ./..., make docs-check, make stdlib-snapshot-check, make self-model, make docs-counts, man-check and changelog check all pass.
  • Fixtures moved to valid bodies: test fixtures that relied on the over-accepted forms were checked as pilot-invalid first, then moved into valid bodies (goldens regenerated): tests/export/testdata/convert/imported_references.sysml (filter in a part def), accept_payload_test.go, event_feature_test.go and runtime/signal_test.go (accept in a part def).

Checklist

  • make test and make lint pass locally
  • Tests added or updated for the change
  • Documentation extended where it already covers the surface (see CONTRIBUTING.md)
  • Changelog entry added as changes/unreleased/<slug>.<section>.md, not as an edit to CHANGELOG.md
  • baselines regenerated and make docs-counts run if a gate count moved (compliance rows need nothing: the census is counted at docs build)
  • No internal work-item labels (waves, slices, F4, K5) in the body, docs, or changelog

Link to Devin session: https://nasa-jpl-demo.devinenterprise.com/sessions/2c39248a684e49a4af9bd168ba90484e
Open in Devin Desktop: https://nasa-jpl-demo.devinenterprise.com/desktop/session/2c39248a684e49a4af9bd168ba90484e?variant=devin
Requested by: @HuiJun

devin-ai-integration Bot and others added 2 commits October 4, 2026 02:35
…es that own them

Co-Authored-By: jason.han <hanhuijun@gmail.com>
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 Bot and others added 2 commits October 4, 2026 02:48
…hen form

Co-Authored-By: jason.han <hanhuijun@gmail.com>
…ies precisely

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

Copy link
Copy Markdown
Contributor Author

End-to-end testing of the CLI, REPL and pilot on ef0f402 (later commits changed only the changelog): all checks passed.

Concern evaluation Default vs. strict notation
Per-element concern verdicts then warning becomes a strict error
Misplaced members and clean controls RDF before and after
Located errors, clean controls isStandard true only from the old declaration
  • Strict validation: views-demo.sysml and examples/self-model report no errors. A misplaced frame, filter or accept gives exactly one error at the member, the same in default and strict mode, and the declarations after it still parse. Valid controls in requirement, concern, viewpoint, package, view, action, calc, case and state bodies are clean.
  • Notation: then y; in a view body is a warning by default and an error under strict; the same line in an action body is silent. if/else there remain parser errors, and the pilot rejects them too.
  • %view: Lander heavyDescender and Robot heavyMockup violate their mass conditions. A custom model gives a violated concern that names the failing element, a concern that holds, an unevaluable concern with no condition, and a conforming view that exposes only light elements.
  • Pilot: the extension libraries report 0 errors and 0 warnings. With self-model and views-demo added: 0 errors (the 46 existing self-model warnings are unchanged). The pilot rejects the invalid fixtures and accepts the controls.
  • RDF: IdentityMetadata is exported as a LibraryPackage with no isStandard (false flags aren't written); the old declaration exported by the same binary gives isStandard true.

Co-Authored-By: jason.han <hanhuijun@gmail.com>
@devin-ai-integration
devin-ai-integration Bot marked this pull request as ready for review October 4, 2026 04:22

@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: No Issues Found

Devin Review analyzed this PR and found no bugs or issues to report.

Devin Review

devin-ai-integration Bot and others added 2 commits October 4, 2026 05:19
…e-conformance

Co-Authored-By: jason.han <hanhuijun@gmail.com>
…e-conformance

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

Copy link
Copy Markdown
Contributor Author

This branch now conflicts with develop. Conflicting files, and the merged PRs that changed them:

To resolve: merge current develop into this branch with an ordinary merge commit (no rebase or force-push).
Don't hand-merge generated files: regenerate docs/project/pilot-differential-baseline.json with go run -C tools ./cmd/pilot-diff -update after the merge.
Regenerate docs/project/pilot-rejection-baseline.json with go run -C tools ./cmd/pilot-reject as described in docs/project/pilot-rejection.md, and the pilot figures quoted in README.md, docs/internals/architecture.md and the testing skills with make docs-counts.

Planned merge order for the view and docs PRs: #889 → #881 → #871 → #882 → #884 → #885 → #886. Each needs these files regenerated again after the one before it merges.

Re-run the full gate (go build ./..., go vet ./..., gofmt -l ., make lint, make docs-check, go test ./...) and wait for green CI before marking ready.

devin-ai-integration Bot and others added 2 commits October 5, 2026 00:18
…e-conformance

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

Copy link
Copy Markdown
Contributor Author

Hold on pushes: please don't push to this branch, including develop merges or empty commits to retrigger CI, until a maintainer says the CI runners are free. Prepare the conflict resolution locally and push it then.

@devin-ai-integration

Copy link
Copy Markdown
Contributor Author

The push pause is lifted for this PR, for one push after the work below.

Merge origin/develop (2f92d69) into this branch. 20 files conflict.

Source files: resolve by hand.

  • internal/semantic/semantics/conformance.go (3 hunks)
  • conformance_test.go (11)
  • internal/check/passes/nonstandard_notation.go (4)
  • internal/frontend/repl/view_test.go (2)
  • examples/views-demo.sysml (1)

Generated files: do not hand-merge. Take develop's side, then regenerate:

  • internal/workspace/libs/stdlib.snapshot: make stdlib-snapshot
  • docs/project/pilot-differential-baseline.json: ./scripts/download-pilot-sysml-validator.sh && ./scripts/download-pilot-kerml-validator.sh && go run -C tools ./cmd/pilot-diff -record
  • docs/project/pilot-rejection-baseline.json: ./scripts/download-pilot-reject-validators.sh && go run -C tools ./cmd/pilot-reject -update

Every row that moves in those two baselines must be adjudicated in pilot-differential.md or pilot-rejection.md, not just recorded. Both files also conflict, with 6 and 5 hunks.

Prose: README.md (4 hunks), docs/project/spec-compliance.md, docs/internals/architecture.md, docs/guide/04-repl.md and six .agents/skills/*/SKILL.md files.

Before pushing, run the full gate and confirm it passes:

  • go build ./..., go vet ./..., gofmt -l . (must print nothing) and go test ./...
  • go test ./tests/corpus with OPENSYSML_REQUIRE_PILOT_CORPORA=1 OPENSYSML_REQUIRE_TRAINING_CORPUS=1
  • make docs-counts
  • make docs

Push once, without force-pushing.

This branch has not been deployed

No deployments
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