For comparison: the mathematical notation datatype does prevent saving invalid edits (though the text input stays disabled, so it’s not a recoverable error because you can’t edit your input anymore to correct it, but that’s another issue):

Had a quick look. Even on the desktop interface for musical notation, the expert is calling wbformatvalue the whole time that you are editing and already getting the error responses, so that is something we can do in wbui2025.

The current implementation in wbui2025 for score is the ScoreValueStrategy (extensions/Score/modules/wikibase.wbui2025/score.wbui2025.entityViewInit.js), which is just a direct subclass of the StringValueStrategy. In that case we are calling wbparsevalue every time the value changes, but not wbformatvalue. But if you look at what the editableTimeSnakValue.vue does, it watches for changes to the text input then hits wbformatvalue via the call to debouncedTriggerFormatAndParse.

It should be possible to create a custom editableScoreSnakValue.vue component in the Score extension, require it in score.wbui2025.entityViewInit.js and wire it into the ScoreValueStrategy by implementing getEditableSnakComponent() (which can return either a string or a reference to a Vue component - we'll return the Vue component). We can then implement a watcher on textvalue as we did in editableTimeSnakValue.vue to trigger the formatvalue call. Once wbformatvalue returns, we can parse the response to see if it's an error, and act accordingly.

To disable the save button, we need to change the definition of 'isIncomplete'. That could be as simple as introducing a haserrors field in the editSnak store (editStatementsStore.js), including haserrors in the evaluation of isIncomplete, and setting haserrors to true if we notice that the result of wbformatvalue includes errors.

There might also be a way to avoid copy-pasting code from editableStringSnakValue to editableScoreSnakValue. Instead of creating a new Vue component, we could add a few more functions to the ValueStrategy interface and make editableStringSnakValue smarter - it could ask the valueStrategy whether the value needs formatting (shouldFormatValueWhileEditing), and have a function for detecting errors (formatValueResponseContainsErrors). In that way we can avoid creating editableScoreSnakValuein the first place and simply implement a couple of new methods in the ScoreValueStrategy. Which also has the advantage that we could re-use it for other validations (e.g. mathematical notation).

The current implementation of mathematical notation avoids this problem because the Math extension validates the values by providing a 'validator-factory-callback' (extensions/Math/src/WikibaseHook.php). In Score (extensions/Score/includes/Hooks.php), this has helpfully been left out:

			'validator-factory-callback' => static function () {
				global $wgScoreMaxLength;
				// load validator builders
				$factory = WikibaseRepo::getDefaultValidatorBuilders();
				// initialize an array with string validators
				// returns an array of validators
				// that add basic string validation such as preventing empty strings
				$validators = $factory->buildStringValidators( $wgScoreMaxLength );
				// $validators[] = new ScoreValidator();
				// TODO: Take out the validation out of Score
				return $validators;
			},

From the version history it's not clear why this is the case or what the plan was here, but I could imagine that calling out to Lilypond is pretty expensive, and doing the validation on a lot of Score properties at save time would make restoring data from dumps prohibitively slow.

Change #1248420 had a related patch set uploaded (by Hasan Akgün (WMDE); author: Hasan Akgün (WMDE)):

[mediawiki/extensions/Wikibase@master] Prevent saving invalid musical notation

https://gerrit.wikimedia.org/r/1248420

Change #1248423 had a related patch set uploaded (by Hasan Akgün (WMDE); author: Hasan Akgün (WMDE)):

[mediawiki/extensions/Score@master] Prevent saving invalid musical notation

https://gerrit.wikimedia.org/r/1248423

I thought the intention behind this task was to prevent these edits server-side, not to make some of our frontends stop trying to save them. AFAICT, the linked patches still allow other API users to save invalid statements.

@Lucas_Werkmeister_WMDE Yep - that's true. I certainly interpreted this as a MEX task because it ended up in this sprint. What's your feeling about adding a ScoreValidator to the callback? Do you think that is practical / sensible from a resource utilisation point of view? And do you think it might be helpful to merge the patches into MEX (or split those off into a MEX ticket) so that we can move forward without ultimately resolving the Wikibase issue for now?

I think resource-wise, adding a validator for musical notation values should be fine, as it would only run when those values are added or changed, which should be pretty rare in the grand scheme of things.

Whether the validation should happen in MEX or for all edits sounds like a product question to me; but if we’re going to do it server side then IMHO we shouldn’t also do it in MEX, that’s just sending unnecessary extra code to the browser.

I agree with @ArthurTaylor because I also thought if it's in the sprint it's MEX, and I'm still on the same page. I think right now MEX itself solves its own problem without any server-side changes, hence our main focus is MEX I'm eager to go with current patches.

Task time notes:
Move to M5

@Alice.moutinho will look into the updated behaviour and create a design for errors that will appear when editors try to publish incorrect syntax for musical notation

This could possibly be a breaking API change

(Just for the record, since I already looked it up: the acceptance criterion “Invalid input can be saved” was originally added in T208489#4927651, presumably during a story time meeting.)

Additional notes: implementing the fix at the API level (per the ticket's original description) would be a breaking change and we would need to notify the community and wait. Once we have the API-level validation, the behaviour will be the same as it is for Math, which is that you get an error message when you click Save (but no indication that anything is wrong until you click save)

@karapayneWMDE, @Arian_Bozorg

Hello team, about this ticket, i need some information to proceed:

Most important:
What can i actually change visually from the way Lylipond spits out errors, what are constranis:

• Does the red box with error have to be like that?
• Does the grey box around the errors have to be like that?
• Does the techy font need to stay like that?
• The text itself -"line 1" "spurious expression (...)" can that be changed?
• the blocks separated by dash lines, does that have to be like that?

Also, what is "Line 1" and "column" referring too, since i dont see any lines or collums? (only necessary to know if i can actually change the text to something understandable - but tbh i am also curious)

ps. we could always give out a general error and simply not save the musical notation if it is not correct (would be the easiest i guess), but i think it would be awesome to help the users fix the error by telling them what is wrong.

We’ve already changed the error message to some extent (with the more recent changes in T412145), maybe that helps to see what our constraints are:

Does the red box with error have to be like that?

No, see above.

Does the grey box around the errors have to be like that?

I guess we could restyle it, but IMHO it should stay this way. This is the boundary between what we can change (in Wikibase) and what we leave alone (Score/LilyPond), and I think that’s a useful distinction for users as well.

Does the techy font need to stay like that?

Same as above, I think – we could change it but I’d rather leave it. (Also, it’s possible that some LilyPond output relies on a fixed-width font, e.g. if it prints out part of the source code and then tries to underline it with the right number of spaces in the next line.)

The text itself -"line 1" "spurious expression (...)" can that be changed?

Nope, that’s LilyPond. (We could try to postprocess it, but IMHO that way madness lies. It might change with a new LilyPond version at any time, and for all I know it might even be localized.)

the blocks separated by dash lines, does that have to be like that?

Same here.