Page MenuHomePhabricator

Cannot call "is BCE" (function which takes a type that references itself)
Closed, ResolvedPublicBUG REPORT

Description

Steps to replicate the issue (include links if applicable):

What happens?:

  • I cannot run the function

What should have happened instead?:

  • It should let me select the value, or preferably reference

Software version (skip for WMF-hosted wikis like Wikipedia):

Other information (browser name/version, screenshots, etc.):
My guess is that the "try this function" widget breaks because of the input type being Gregorian era, as defined here: https://wikifunctions.beta.wmflabs.org/wiki/Z10528

Gregorian Era is a so called nominal, a type that has a key that of its own type (i.e. Z10528 has one key, Z10528K1 of type Z10528). The UX tries to turn that into a literal, which in turn tries to create a literal, etc. This recursion breaks.

What we should do instead is that whenever we get to an argument of a recursive type, i.e. of a type that has a key that is itself, we should, instead of a literal, ask for a reference of that type.

I.e. what we should see here for the argument is a field which is a ZObject selector bound to the given type, i.e. to Gregorian Era.

Event Timeline

When a type has a key that points to itself, we should per default show a reference, not a literal

I will try to work on this since we had the same problem in the backend.

Can someone point me to the code in the UX that ends up turning the nominal into a literal? I think we can implement some similar functionality to what exists in the orchestrator.

The alternative would be to try to offload some of this work by actually calling the orchestrator, but I fear that that won't really solve the problem (not to mention the performance issues that such a solution would introduce).

Change 939357 had a related patch set uploaded (by Cory Massaro; author: Cory Massaro):

[mediawiki/extensions/WikiLambda@master] Do not create a generic type when type has already been seen; default to a Z9/Reference instead.

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

Jdforrester-WMF renamed this task from Cannot call "is BCE" to Cannot call "is BCE" (function which takes a type that references itself).Jul 18 2023, 6:26 PM

Change 939357 merged by jenkins-bot:

[mediawiki/extensions/WikiLambda@master] Do not create a generic type when type has already been seen; default to a Z9/Reference instead.

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

The function can now be called. It returns true (correctly) on this input:

{
  Z1K1: 'Z10528',
  Z10528K1: 'Z10529'
}

but false (arguably incorrect) on this one:

{
  Z1K1: 'Z10528',
  Z10528K1: {
    Z1K1: 'Z10528',
    Z10528K1: 'Z10529'
  }
}

The function can now be called. It returns true (correctly) on this input:

{
  Z1K1: 'Z10528',
  Z10528K1: 'Z10529'
}

but false (arguably incorrect) on this one:

{
  Z1K1: 'Z10528',
  Z10528K1: {
    Z1K1: 'Z10528',
    Z10528K1: 'Z10529'
  }
}

Any state that is easily triggerable through the front-end seems to return the correct behavior, see the screenshots:

Screenshot 2023-07-19 at 3.39.02 PM.png (1×944 px, 69 KB)

Screenshot 2023-07-19 at 3.38.02 PM.png (1×936 px, 65 KB)

Screenshot 2023-07-19 at 3.38.17 PM.png (1×940 px, 66 KB)

Screenshot 2023-07-19 at 3.39.43 PM.png (666×934 px, 38 KB)

Screenshot 2023-07-19 at 3.40.08 PM.png (674×940 px, 39 KB)

Screenshot 2023-07-19 at 3.38.46 PM.png (878×938 px, 52 KB)

Screenshot 2023-07-19 at 3.39.14 PM.png (886×948 px, 53 KB)

Screenshot 2023-07-19 at 3.38.34 PM.png (1×946 px, 69 KB)

Screenshot 2023-07-19 at 3.39.33 PM.png (882×942 px, 55 KB)

Screenshot 2023-07-19 at 3.39.59 PM.png (884×936 px, 55 KB)

The one way I can trigger the wrong behavior is like the following:

Screenshot 2023-07-19 at 3.41.05 PM.png (1×940 px, 96 KB)

I think that's good enough for now.

I want to file a follow up task, but then close this one, to further improve the behavior. But it doesn't break, so that is awesome! Thank you!

I am closing this bug as resolved, since "is BCE" now works, can be called, and doesn't badly break the UX anymore. Yay!

I am opening a follow-up task to streamline the interface a bit: T342315