Skip to content

Put the OMEX validation errors in the exception message - #1827

Merged
jcschaff merged 1 commit into
masterfrom
fix/omex-validation-message
Aug 6, 2026
Merged

Put the OMEX validation errors in the exception message#1827
jcschaff merged 1 commit into
masterfrom
fix/omex-validation-message

Conversation

@jcschaff

@jcschaff jcschaff commented Aug 4, 2026

Copy link
Copy Markdown
Member

Symptom

The nightly regression reports OMEX validation failures like this:

[ERROR] SEDMLExporterSBMLTest.test_sedml_roundtrip_SBML:348
          ->SEDMLExporterCommon.sedml_roundtrip_common:183 » OmexValidation

That is the entire diagnostic. Nothing about which model, which check, or why.

Cause

OmexValidationException built its detail only in toString() and never passed
it to super, so getMessage() was null:

public OmexValidationException(List<OmexValidationError> errors) {
    this.errors = errors;          // no super(message)
}
public String toString() { /* builds the detail here */ }

JUnit, surefire and ordinary logging all report getMessage(). The errors were
present in the object the whole time, just unreachable by everything that prints
exceptions.

This also silently degraded SEDMLExporterCommon, which logs e.getMessage()
when deciding whether a fault is already recorded — those lines have been
printing null.

What was hiding behind it

Recovered from the uploaded surefire XML for run 30796032928. These are specific
and actionable, not environmental:

biomodel_59361239.vcml   XPath …/sbml:species[@id='Src_plasmamembrane']/@initialConcentration
                         does not match any elements of model `full_model`
biomodel_147699816.vcml  XPath …/sbml:parameter[@id='bounBRaf_frac']
                         does not match any elements of model `_sorafenib`
biomodel_28625786.vcml   Each identified SED object must have a unique id.
                         Multiple objects have the following ids: [compartmental]

The exported SED-ML references entities by XPath that are absent from the
exported SBML. Those export defects are not addressed here — this PR only
makes them visible. They have been failing every nightly in the retained history
(10 of 10 runs, back to 2026-07-25) and all sit in the slowModels set, so they
never run in the merge gate or in a plain workflow_dispatch.

Change

Build the message in the constructor and pass it to super. The toString()
override becomes redundant and is dropped, so stack traces show the standard
class: message form instead of a bare message with no exception type.

CI will now print the model, the failing XPath and the error type directly.

Tests

New OmexValidationExceptionTest (4 cases): the message is non-null and contains
every error and its type, toString() still carries the detail, the errors
list stays available for callers that branch on error type, and an empty list
still yields a message. Full vcell-core Fast group passes (437).

🤖 Generated with Claude Code

https://claude.ai/code/session_01SvqmME7MkRUNEYje5HpiLt

OmexValidationException assembled its detail only in toString(), never passing it
to super, so getMessage() returned null. JUnit, surefire and callers all report
getMessage(), so a validation failure surfaced in CI as:

    SEDMLExporterSBMLTest.test_sedml_roundtrip_SBML:348 » OmexValidation

with no indication of what failed. The nightly regression has been red for ten
consecutive nights partly on these, and the actual errors were only recoverable
by downloading the surefire XML artifact and reading the stack-trace text. They
turn out to be specific and actionable, e.g.

    XPath /sbml:sbml/sbml:model/sbml:listOfSpecies/sbml:species[@id='Src_plasmamembrane']
    does not match any elements of model `full_model`

The same null also reached SEDMLExporterCommon, which logs e.getMessage() when
deciding whether a fault is already known - those lines have been printing
"null".

Build the detail in the constructor and hand it to super. The toString() override
is now redundant and dropped, so stack traces get the standard "class: message"
form rather than a bare message with no type.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01SvqmME7MkRUNEYje5HpiLt
@jcschaff

jcschaff commented Aug 4, 2026

Copy link
Copy Markdown
Member Author

Verified against the real failing test, not just the unit test

Worth stating plainly: this repo's PR CI does not exercise this code path. These
are SEDML_SBML_IT tests, which run only in regression.yml, and the failing cases
are additionally gated behind test.include.slow. So the green checks on this PR say
nothing about the fix. It was run locally instead.

biomodel_59361239.vcml, the real failing case, run with
-Dgroups=SEDML_SBML_IT -Dtest.include.slow=true (104s):

Before — the entire diagnostic:

[ERROR] SEDMLExporterSBMLTest.test_sedml_roundtrip_SBML:349
          ->SEDMLExporterCommon.sedml_roundtrip_common:183 » OmexValidation

After — same run, same failure:

[ERROR] SEDMLExporterSBMLTest.test_sedml_roundtrip_SBML:349
          ->SEDMLExporterCommon.sedml_roundtrip_common:183 » OmexValidation
          OMEX VALIDATION FAILED with 1 error(s)
OMEX_PARSE_ERROR: ["The SED-ML file at location `./SBML.biomodel_59361239.vcml.sedml` is invalid.",
  [["Task `repTsk_0_2_0` is invalid.",[["Changes are invalid.",[["Change 1 is invalid.",
  [["XPath `/sbml:sbml/sbml:model/sbml:listOfSpecies/sbml:species[@id='Src_plasmamembrane']/@initialConcentration`
     does not match any elements of model `full_model`."]]]]]]],
   … 27 further tasks, across models full_model, individual_knockouts,
     individual_knockouts_23, time_delay_60_s, receptor_density …

The failure itself is unchanged — this only makes it legible.

One wart, left as is

The count says "1 error(s)" while the text lists 28 invalid tasks. That is accurate to
the data structure: the Python validator returns all SED-ML problems bundled in a
single parse_errors entry, so errors.size() is 1. Making the count reflect
individual problems would mean parsing that nested JSON, which is a bigger change than
restoring a message, and the detail is all present either way.

@jcschaff
jcschaff merged commit b3f9e97 into master Aug 6, 2026
9 checks passed
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