fix: reject names the expression parser cannot read (#2062)

[jcschaff]

Fixes #2062.

What was wrong

Name validation and the expression grammar disagreed on what an identifier is:

• validation used Character.isJavaIdentifierStart/Part, which accept any Unicode letter
• Parser.jjt defines #LETTER as ["a"-"z", "_", "A"-"Z"] — ASCII only

So a species named PROTEÍNA_A was accepted, saved, and then failed math generation with
Parse Error while parsing expression 'PROTEÍNA_A' → "Application has no generated Math".
The error names an expression, so nothing pointed at the species. Three Contact-Us crash
reports from one Spanish-language model.

The root cause is narrower than the issue says

Model.validateNamingConflicts already required a legal identifier and already produced a
good message with a suggested replacement:

if(!newSymbolName.equals(TokenMangler.fixTokenStrict(newSymbolName))){
    throw new ModelPropertyVetoException(... " not legal identifier, try '" + ... + "'.", e);
}

The guard was correct in shape. It never fired because fixTokenStrict used
Character.isLetterOrDigit, so it considered Í a letter and returned the name unchanged. The
manglers that exist precisely to turn external names into legal identifiers were passing
non-ASCII letters straight through.

I've updated the issue with this correction.

The fix

1. TokenMangler gains one definition of the rule — isValidExpressionIdentifier /
   indexOfFirstIllegalIdentifierChar — and fixToken / fixTokenStrict now use it.
   Behaviour is unchanged for any name that is already ASCII, i.e. every name these manglers
   previously handled correctly.
2. Model.validateNamingConflicts asks that predicate directly, so the existing guard now
   works for every model symbol, not just species contexts. This is what actually fixes the
   reported bug.
3. SpeciesContext.vetoableChange gets the same rule, and its message now states what a
   name may contain instead of only naming the offending character.

Keeping already-saved models openable

Models saved with such a name are unrunnable today, but they do open — and opening them is
the user's only way to rename the species. Enforcing the rule on the read path would have taken
that away, turning "opens but won't run" into "won't open", which is worse than the bug.

Reads from VCML and from the database therefore go through
SpeciesContext.fromPersistedContent / Model.setRestoringFromPersistedContent, which relax
the lexicon check for the duration of the read only — naming conflicts are still checked. The
illegal name is then reported by SpeciesContext.gatherIssues as an error the user can act on.

I found this the right way: the round-trip test failed, which is also how I found the
Model-level guard I'd missed on the first pass.

Tests

New SpeciesContextNameTest (10 tests, Fast). The central one asserts the rule and the parser
agree by actually parsing — every accepted name must parse as a single identifier equal to
itself, every rejected name must not — so tightening or loosening either side alone fails here.
That is the check whose absence let the two drift apart.

It also covers the real round trip (a VCML holding an unreadable name still loads, keeps the
name, and is reported as an error issue) and records one thing worth knowing: . is the single
character the grammar accepts inside an IDENTIFIER — as a name-scope qualifier,
(<ID> ".")* <ID> — that a name still may not contain.

Verification

• SpeciesContextNameTest — 10/10
• vcell-core Fast — 606 tests, 1 error, and that one is the documented Poetry/VCellDataTest
  environmental failure, present before this change
• vcell-util Fast 42/42 · vcell-math Fast 6545/6545 · vcell-client Fast 31/31
• full mvn compile test-compile across all modules — clean

vcell-server Fast was still running locally when I stopped it; every class it had reported was
green. Worth watching the SBML/SEDML regression suites in the merge queue, since re-arming the
mangler could change how an imported model with non-ASCII names is renamed on import — that is
the intended behaviour, but it is the one place the blast radius extends past the reported bug.

🤖 Generated with Claude Code

https://claude.ai/code/session_01D71LBYmQNf5J94wPqr81Jx