[import] Report the syntax errors truncating a textual import

[1wgrumph]

Fixes #2510.

File uploads now preserve the model the parser produced and include syntax diagnostics in the import report. Validation failures also leave the model available. Standard-library resolution errors are ignored, and text-editor behavior is unchanged.

The original bug is reproduced on unmodified upstream code. The same regression test passes with this change. All 14 required build modules pass, including 50 import-module tests and Checkstyle.

Changelog and release notes target 2026.11.0. Diagnostic detection follows the embedded CLI’s format and ordering, which need rechecking when that dependency changes.

PLEASE READ ALL ITEMS AND CHECK ONLY RELEVANT CHECKBOXES BELOW

Auto review

• Have you reviewed this PR? Please do a first quick review, It is very useful to detect typos and missing copyrights, check comments, check your code... The reviewer will thank you for that :)

Project management

• Has the pull request been added to the relevant milestone?
• Have the priority: and pr: labels been added to the pull request? (In case of doubt, start with the labels priority: low and pr: to review later)
• Have the relevant issues been added to the pull request?
• Have the relevant labels been added to the issues? (area:, type:)
• Have the relevant issues been added to the same project milestone as the pull request?

Changelog and release notes

• Has the CHANGELOG.adoc + doc/content/modules/user-manual/pages/release-notes/YYYY.MM.0.adoc been updated to reference the relevant issues?
• Have the relevant API breaks been described in the CHANGELOG.adoc?
• Are the new / upgraded dependencies mentioned in the relevant section of the CHANGELOG.adoc?
• In case of a change with a visual impact, are there any screenshots in the doc/content/modules/user-manual/pages/release-notes/YYYY.MM.0.adoc?
• In case of a key change, has the change been added to Key highlights section in doc/content/modules/user-manual/pages/release-notes/YYYY.MM.0.adoc?

Documentation

• Have you included an update of the documentation in your pull request? Please ask yourself if an update (installation manual, user manual, developer manual...) is needed and add one accordingly.

Tests

• Is the code properly tested? Any pull request (fix, enhancement or new feature) should come with a test (or several). It could be unit tests, integration tests or cypress tests depending on the context. Only doc and releng pull request do not need for tests.

[1wgrumph]

Pushed a follow-up commit repairing three defects found while re-reading the first one, plus the wording of the public texts.

The validation pass could deadlock, and its timeout was unreachable. The CLI serializes an AST on its standard output during that pass for every document it finds valid, that pipe was never drained, and the standard error was joined before the timed wait. A document large enough to fill the pipe blocked the upload forever instead of hitting the bound. The output of the pass is now redirected to DISCARD, the process is waited for before its report is read, termination is awaited after destroyForcibly(), and both CLI invocations of a conversion share a single 60 second bound instead of one each.

Substring matching could reject a valid document. The CLI renders a diagnostic as line <number>: <message> [<source text>], so an unrelated diagnostic quoting a source range that contains Expecting end of file satisfied both filters, for example line 3: A Feature must be typed by at least one type. [attribute msg = "line 1: Expecting end of file";]. The diagnostic is now matched structurally, anchored on the message as ^line \d+: Expecting end of file\b.

An inconclusive pass silently restored the old behaviour. A timeout, a launch failure or a failed read all returned an empty list, which the caller read as "the document is complete", so the truncated AST was accepted. The pass now has three outcomes: complete, truncated, and unavailable. The AST is withheld and an operational error is reported for the last one, and the unrelated diagnostics are only ignored after a pass that actually completed.

Tests. Every case importing a document now asserts that the B definition declared after the syntax error is present in the returned AST (and ro for the recovered case), rather than only that an AST was returned. Three cases were added: a valid document whose source text is quoted back by a diagnostic containing Expecting end of file, an injected CLI stub writing more than a pipe buffer during the validation pass, and an injected CLI stub whose validation pass never terminates. The two stub cases fail against the previous commit (30 s hang, then a withheld AST) and the marker case fails against its substring rule. The skip-when-no-Node guard is kept and its reason is now documented.

Texts. The claim that the validation pass "does not serialize the AST" was wrong and has been removed from the issue, this description and the release note: the pass does serialize one for a validation-clean document, and therefore does pay the JSON.stringify cost of #2042 for it. The scope is now described as syntax errors that leave an unparsed tail, and the associated parser diagnostics, rather than all syntax errors or all incomplete ASTs. The issue now records the baseline (bb52f47), the embedded SysIDE CLI version (0.9.0), the Node version (v22.23.2), the corpus and the exact command.

mvn -pl backend/application/syson-sysml-import -am test is green (50 tests, 6 in SysmlToAstTest), checkstyle reports no new violation.

[1wgrumph]

Startup regression from my previous commit, fixed in aaafa7d.

The commit repairing the validation pass added a second, package-private constructor so the tests could shorten the timeout. Spring only selects a constructor implicitly when a class declares exactly one, so SysmlToAst stopped being instantiable and the application failed to start:

Error creating bean with name 'sysmlToAst': Failed to instantiate [org.eclipse.syson.sysml.SysmlToAst]: No default constructor found
Caused by: java.lang.NoSuchMethodException: org.eclipse.syson.sysml.SysmlToAst.<init>()

The constructor taking the configured CLI path is now annotated with @Autowired, which restores the previous wiring.

Added SysmlToAstBeanTest, which creates the bean in an AnnotationConfigApplicationContext and asserts getBean(SysmlToAst.class) succeeds, so a future constructor cannot break the startup unnoticed. It reproduces the failure above against the parent commit. It is a separate class on purpose: it does not run the parser, so it must not sit behind the guard that skips SysmlToAstTest when no Node runtime is available.

mvn -pl backend/application/syson-sysml-import -am test is green (51 tests), checkstyle reports no violation.

[1wgrumph]

Second review round, fixed in 225e2f6. Reading the validation report turned out to be the fragile part of this change, so I treated it as a parsing problem in its own right: getting it wrong either rejects a valid document or, worse, lets a truncated one through.

Abnormal termination was accepted as "document complete". A --validate pass that crashed or was killed prints no diagnostic, and the previous commit read the absence of the sentinel as success. The exit code is now part of the decision: 0 means nothing was reported; 1 is accepted only together with the There are validation errors: line the CLI prints before its diagnostics, which is what tells a reported validation apart from a CLI that failed to run; any other exit leaves the outcome unknown and the AST is withheld.

Coloring could hide the sentinel. With FORCE_COLOR set in the server environment the CLI wraps every line in escape sequences, so no diagnostic matched and a truncated document would have been imported silently. FORCE_COLOR=0 and NO_COLOR=1 are now set for both invocations. FORCE_COLOR is the one that decides: with NO_COLOR=1 alone the CLI prints Warning: The 'NO_COLOR' env is ignored due to the 'FORCE_COLOR' env being set and colors anyway.

Diagnostics are now split statefully. The CLI copies the source range verbatim, newlines included, so a continuation line can look exactly like a diagnostic header. A line only starts a new diagnostic when the brackets of the previous one are closed. Where that cannot be decided — the words appear at a line start, no diagnostic header carries them, and the brackets of the report do not balance — the outcome is unavailable rather than clean, so the ambiguity fails closed.

The report is bounded, and cleanup is tree-wide. The stream is still drained to its end so the CLI can terminate, but at most 64 diagnostics are retained, each abbreviated to 1 KiB; the parsing pass keeps at most 64 KiB of its own stderr. A pass that has to be destroyed is now destroyed together with descendants(), captured before the root is killed, under a bounded cleanup wait — otherwise a CLI that spawned a child left it running with the pipes it inherited after convert returned.

Tests. Seven added, all in SysmlToAstTest: convertDocumentWithCrashingValidation, convertDocumentWithUnrecognizedValidationFailure, convertDocumentWithColoredValidationReport, convertDocumentWithMultiLineQuotedSourceRange, convertDocumentWithUnbalancedQuotedSourceRange, convertDocumentWithVeryManyDiagnostics, convertDocumentWithValidationDescendant. Each was checked against the unrepaired logic by reverting one behaviour at a time, and each time exactly the matching test or tests failed and nothing else did: dropping the exit-code check fails the two termination tests, dropping the forced environment fails the coloring test, dropping the bracket state fails both quoted-range tests, ignoring the bracket balance fails the unbalanced one, removing the caps fails the bounded-report test, and killing only the root fails the descendant test.

Texts. The issue and this description now say the extra parse is one per SysmlToAst.convert call — upload, New object from text and the expression editor — rather than "only for the file being uploaded", and the timeout is described as reached before the stderr result is interpreted, since stderr is deliberately drained concurrently.

mvn -pl backend/application/syson-sysml-import -am test is green (58 tests, 13 in SysmlToAstTest), checkstyle reports no violation.

One limitation worth stating: the CLI's report is a human rendering with unescaped source text, so the bracket balance is a heuristic, not a grammar. It is deliberately biased towards withholding the AST when it cannot decide. A machine-readable diagnostic output from the CLI would remove the guesswork entirely, if that is something the SysIDE side would consider.

[1wgrumph]

One more, in dc9e0d0: GeneralPurposeTests.checkJavaCode was failing on my own test helper.

The helper reading the returned AST wrapped the stream failure in an UncheckedIOException, and this repository rejects throw new XXXException outside the files it whitelists. The helper and the three tests calling it declare IOException instead, like the surrounding tests already do.

This was in the branch since the first repair commit. My earlier runs built the module with -pl backend/application/syson-sysml-import -am but ran the tests with -pl alone, so syson-tests was never executed and the convention check never ran locally. mvn -pl backend/application/syson-sysml-import -am test is now green end to end: 58 tests in this module, 3 in GeneralPurposeTests, no failure anywhere in the reactor, and checkstyle reports no violation.

[AxelRICHARD]

@1wgrumph thank you for providing this PR.

Please take a look at previous commits, and at https://doc.mbse-syson.org/syson/v2026.7.0/developer-guide/index.html#_contribute_a_change_in_the_codebase.
You will see how to format your commit message.
I would also prefer only 1 or 2 commits, not 14 :)

We are also close to the 2026.9.0 release (this Wednesday), so this PR will be part of 2026.11.0 release.

Thank you for your understanding.

Regards,

[1wgrumph]

Reshaped into one commit in the format from the developer guide ([2510] subject, Bug: and Signed-off-by: trailers); the tree is unchanged from the previous head. Understood on the 2026.11.0 target: the changelog and release-note entries currently sit under 2026.9.0, and I will move them once the 2026.11.0 section exists.

[AxelRICHARD]

Some new remarks:

1. The diagnostic parser is not robust. It infers diagnostic nesting from raw [ / ] characters in verbatim SysML source text (backend/application/syson-sysml-import/src/main/java/org/eclipse/syson/sysml/SysmlToAst.java:422). Brackets can naturally occur inside comments or string literals, so depth (line 443 (backend/application/syson-sysml-import/src/main/java/org/eclipse/syson/sysml/SysmlToAst.java:443)) is not a reliable framing mechanism. A crafted but valid document can be falsely rejected—or, in a multiline case, misclassify quoted content as a real diagnostic. The tests validate the implementation’s invented grammar, not the CLI’s actual escaping/framing contract.
   In in backend/application/syson-sysml-import/src/main/java/org/eclipse/syson/sysml/SysmlToAst.java, mainly:

• readValidationStdErr(...) — reads and splits the validation report.
• Local variable depth — decides whether a line starts a new diagnostic.
• BoundedLine.bracketBalance / BoundedLineReader — calculate depth by counting raw [ and ].
  The fragile part is: depth += line.bracketBalance(); at backend/application/syson-sysml-import/src/main/java/org/eclipse/syson/sysml/SysmlToAst.java:443.
  It assumes brackets in the CLI’s verbatim quoted SysML source delimit diagnostic content. But brackets can occur inside actual SysML strings/comments/source, so they are not reliable framing tokens.

2. The patch is far too broad for the bug. It adds 715 production lines and 607 test lines to add one validation call. Process-tree sampling, forced descendant termination, custom bounded line reading, interruption choreography, and 4 MB diagnostic tests are a second project. That raises regression risk in all upload, text-object, and expression-editor paths, while the PR admits every conversion is now roughly doubled in parser cost.
   The PR adds a custom stream parser specifically so a CLI diagnostic containing a huge quoted source range is drained without ever materializing the whole physical line:

• The test generates 64 KiB × 64 = 4 MiB of one-line diagnostic payload.
• BoundedLineReader (backend/application/syson-sysml-import/src/main/java/org/eclipse/syson/sysml/SysmlToAst.java:730) reads character-by-character, retains only 1,024 characters, and separately counts brackets across the discarded remainder.
• That machinery exists largely to satisfy the 4 MB test cases in SysmlToAstTest (backend/application/syson-sysml-import/src/test/java/org/eclipse/syson/sysml/SysmlToAstTest.java:340).

So “a second project” means the patch goes beyond “run validation and detect truncation” into designing a bounded, multiline diagnostic protocol parser plus adversarial process cleanup.
There is a legitimate security/availability argument for it: user-controlled SysML may be echoed by the CLI, and diagnostics should not permit unbounded memory use. But it needs an explicit requirement or evidence that SysIDE actually emits multi-megabyte source ranges. Otherwise it is speculative complexity—and it still relies on raw bracket counting, which is not a safe protocol parser for arbitrary source text.

3. Fail-closed availability is a huge product change. Any validation CLI failure, timeout, changed output format, or unrecognized exit now rejects a document that previously imported.
   After thinking about it, i think we should not change this behavior: a model should still be imported even if some of its lines are not. But the report file should contains the information that some data has been lost during the import.

[AxelRICHARD]

Hello @1wgrumph for your information, the 2026.9.0 has been released.

Regards,

[1wgrumph]

Thanks, Axel. Updated for 2026.11.0. Partial imports are preserved, and syntax diagnostics appear in the import report.

I removed the bracket parsing and extra process-management code, limited the additional validation pass to file uploads, and reduced the tests to six focused cases. The original bug is reproduced, and the revised import suite passes.

The workflows are awaiting maintainer approval. Could you approve them and take another look?

[AxelRICHARD]

Hello @1wgrumph,

All my comments seem to have been taken into account, thank you very much!
I will test it a last time, if everything is fine I will rebase and merge the PR. Otherwise I will let you know If some other changes are required.

Thank you again for all your work!

Regards,