Click link for reprodution: https://test.wikidata.org/w/index.php?action=edit&title=Lexeme:L855&undo=707360&undoafter=707359

Better STR:

• Create a new Lexeme (L123)
• Add a Form (L123-F1)
• Delete the Form
• Restore the revision of the Lexeme that includes the Form
• 'Undo' the deletion of the Form

From discussion with @Lucas_Werkmeister_WMDE:

'One thing to keep in mind is that (AFAIK) the form/sense counter in a lexeme is preserved across undo/restore. So no matter what you do, it should be impossible to create a new form L123-F1 that’s independent of the original one (any newly created form should be at least -F2 even if you’ve deleted F1 and/or restored revisions in the meantime) and as a consequence, I think in this case it should be sound to conclude that the L123-F1 being restored (undo deletion) is “the same” form as the one that’s already present on the lexeme. In the case you described, that means it would be fine to say “nothing to do here”.

More generally, I assume it’s possible that the form being restored differs from the one already present (e.g. has additional statements), so it might make sense to try to merge it but then again, I’m not sure if that’s actually useful or not. Perhaps it would be better to just always say something like “L123-F1 already exists (again) so its deletion cannot be undone”.'

@Arian_Bozorg: What do you want as behaviour here in the case that we try and restore a form that already exists on the lexeme (with the possibility that the form being restored might contain different statements to the form that exists on the lexeme)?

Change #1156281 had a related patch set uploaded (by Arthur taylor; author: Arthur taylor):

[mediawiki/extensions/Wikibase@master] Handle ConflictException when undoing and already-undo diff

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

Change #1156286 had a related patch set uploaded (by Arthur taylor; author: Arthur taylor):

[mediawiki/extensions/WikibaseLexeme@master] Ensure PatcherException is thrown in the event of patching conflict

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

Change #1159352 had a related patch set uploaded (by Arthur taylor; author: Arthur taylor):

[mediawiki/extensions/WikibaseLexeme@master] Drop form additions from patches that try to add an existing form

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

Change #1156286 abandoned by Arthur taylor:

[mediawiki/extensions/WikibaseLexeme@master] Ensure PatcherException is thrown in the event of patching conflict

Reason:

Going with a different approach I68bc73d5

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

Change #1156281 abandoned by Arthur taylor:

[mediawiki/extensions/Wikibase@master] Handle ConflictException when undoing and already-undo diff

Reason:

Going with a different approach I68bc73d5

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

Change #1159464 had a related patch set uploaded (by Arthur taylor; author: Arthur taylor):

[mediawiki/extensions/Wikibase@master] Add additional argument to patchEntity to allow soft-fail

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

Tried a couple of different approaches here. This seems like a tricky problem to solve in an elegant way given the way the interfaces are currently written and given the expectations of existing consumers of the WikibaseLexeme API.

Specifically, wbladdform is expected to throw the ConflictException in the case that a user tries to add the same Form to a Lexeme twice. We don't want to change this behaviour - there may be API clients that query the API to list forms, then try and add missing forms. The ConflictException allows those clients to implement behaviour in a reliable way.

The current patches (1159464-PS2, 1159352-PS6) add an extra argument to patchEntity to allow callers to decide if conflicts should fail with an exception or not (making not throwing the exception the default). But @Lucas_Werkmeister_WMDE has reservations with the current (master) implementation of wbladdform that I will let Lucas explain.

Some things I learned about Wikibase(Lexeme) today:

1. A very similar error as in this task can also be produced by the wbladdform API (though it’s reported as a RuntimeException rather than ConflictException), by trying to add a form with a baserevid that pre-dates the latest form addition. For example, use this API sandbox link, let the API sandbox fill in the token, and submit it: you’ll get an error “Edit conflict: At least two forms with the same ID were provided: L1542-F1”.

2. The WikibaseLexeme user interface doesn’t send a baserevid. (Otherwise, we’d presumably have noticed the above error sooner.) We should still fix the API, but it’s not going to affect the user interface.

3. wbladdform / AddForm and wbladdsense / AddSense misuse the Wikibase EditEntity interface, as far as I can tell. (This is the reservation of mine that Arthur referred to.) Both of them always start with the latest lexeme revision and add the form/sense to that – the base revision ID is only passed down into the EditEntity (or rather, the EditEntityFactory). But this is not the correct way to use EditEntity – you should start with the entity revision specified by the baserevid, and EditEntity is responsible for checking whether this is the latest revision and potentially trying to resolve an edit conflict by diffing the edit against the base revision and applying that patch to the latest revision. What happens now is that EditEntity diffs the edited latest revision (as supplied by AddForm) against the base revision (picking up any number of unrelated changes in between) and then tries to apply that patch to the latest revision, which makes no sense. (For other code that uses EditEntity correctly, see mainly EntitySavingHelper::loadEntity(), as well as the WikibaseLexeme APIs wblremoveform / RemoveForm, wblremovesense / RemoveSense, wbleditformelements / EditFormElements, and wbleditsenseelements / EditSenseElements.) I think this bug has been present since Stop using using EntitySavingHelper in AddForm API module (October 2017); unfortunately there’s no mention in the change of why we stopped using EntitySavingHelper. I think I would like to fix this issue first – thinking about the details of diffing, patching, base revisions, edited entities, latest revisions will hopefully be easier once those terms don’t have different meanings in half the code base.

4. Both MediaWiki and Wikibase handle self-conflicts different from edit conflicts with another user (see MediaWikiEditEntity::fixEditConflict() for the Wikibase part). For now, this is just something to keep in mind when we work with and test this code. (There’s a discussion about changing the MediaWiki behavior in T175745, in which case Wikibase should probably lose this weird special case as well – if not sooner, IMHO.)

Change #1162028 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Lucas Werkmeister (WMDE)):

[mediawiki/extensions/WikibaseLexeme@master] Use baserevid correctly in wbladdform+wbladdsense

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

Change #1162028 merged by jenkins-bot:

[mediawiki/extensions/WikibaseLexeme@master] Use baserevid correctly in wbladdform+wbladdsense

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

Change #1164131 had a related patch set uploaded (by Arthur taylor; author: Arthur taylor):

[mediawiki/extensions/WikibaseLexeme@master] Update buildSaveFlags to implement correct behaviour

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

Change #1164230 had a related patch set uploaded (by Lucas Werkmeister (WMDE); author: Lucas Werkmeister (WMDE)):

[mediawiki/extensions/WikibaseLexeme@master] WIP: Ignore existing form/sense in LexemePatcher

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

I think we can use the diff on the next form/sense ID to distinguish between the “undo” case (ignore conflict) and the “add form” case (report conflict, don’t save)?

If we’re undoing the deletion of a form, then the diff includes adding a form, but does not change the next form ID, because deleting the form didn’t change the next form ID either.

If we’re adding a form, then the diff includes both adding a form and incrementing the next form ID.

If those assertions are also true in the opposite direction – form addition without changing next form ID implies undo deletion, and form addition with changing next form ID implies genuine addition – then LexemePatcher can use this to decide what to do about conflicting forms, without needing any more information elsewhere.

Updated the change with that idea. It seems to make both the “undo” test and the “add form” test green, at least.

Change #1164230 merged by jenkins-bot:

[mediawiki/extensions/WikibaseLexeme@master] Skip restoring already-restored form/sense

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

on beta I was unable to delete the form
https://www.wikidata.beta.wmcloud.org/wiki/Lexeme:L57549

ArthurTaylor added a comment.Edited · Jun 3 2025, 13:44
Comment Actions

STR:

• Create a new Lexeme (L123)
• Add a Form (L123-F1)
• Remove the Form
• Get error :

I just clicked the delete button and it disappeared. Could have been a race condition if you made the addition immediately before the deletion?

Change #1159352 abandoned by Arthur taylor:

[mediawiki/extensions/WikibaseLexeme@master] Drop form additions from patches that try to add an existing form

Reason:

See preferred approach I65f26a50fc7a136a2376bb94d08e5942872c4173

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

Change #1164131 abandoned by Arthur taylor:

[mediawiki/extensions/WikibaseLexeme@master] Update buildSaveFlags to implement correct behaviour

Reason:

See preferred approach I65f26a50fc7a136a2376bb94d08e5942872c4173

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

Change #1159464 abandoned by Arthur taylor:

[mediawiki/extensions/Wikibase@master] Add additional argument to patchEntity to allow soft-fail

Reason:

See preferred approach I65f26a50fc7a136a2376bb94d08e5942872c4173

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

According to Logstash, this error hasn’t happened again since 30 June 2025 (1.45.0-wmf.7):