This is immensely helpful; thank you!

Since this is now working in prod, I guess we can close it?

In T428835#12011531, @Jdforrester-WMF wrote:

After the deploy, I've now edited https://www.wikifunctions.org/view/en/Z23166 and it's passed its tests. https://www.wikifunctions.org/view/en/Z27336 also looks to have updated its cache.

What's the next set of issues to fix?

This isn't Rust's fault. It's this: https://phabricator.wikimedia.org/T428917

Partial fix will be here: https://gitlab.wikimedia.org/repos/abstract-wiki/wikifunctions/function-orchestrator/-/merge_requests/714.

In T428835#12007984, @99of9 wrote:

@cmassaro I hope you don't mind if I tag you into this. I think something near this is causing us major problems today.

Unassigning since this is no longer valid.

The necessary functions have now been confirmed to work with the v2 abstractions (in separate tasks).

I do want to re-raise the issue of a builtin n-ary Apply/Map/Reduce series, because that would clean up a lot of this (and make the Reduce-based apply2 unnecessary).

Yes, this is nasty. The issue is documented here: https://phabricator.wikimedia.org/T428475 .

When I run this with Ecuador, I get a Z579: 32. Looks like we're not configuring the default concurrent call rate limit, and I'm not quite sure why this function tries to make so many concurrent evaluator calls. This error, by the way, is not labelled correctly.

In T419774#11982951, @Jdforrester-WMF wrote:

Is this now done?

Benchmarks are looking a bit better after all of the asZObject changes.

In T426336#11986573, @Jdforrester-WMF wrote:

Should we leave this task in In Engineering for a while in case we see the log in prod?

In T409229#11982473, @Jdforrester-WMF wrote:

@cmassaro: Do you think we need to get this done this Quarter as part of v2 follow-up?

So, to answer your question, it depends on what we decide tomorrow!

In T416142#11982458, @Jdforrester-WMF wrote:

@cmassaro: Do you think we need to get this done this Quarter as part of v2 follow-up?

Good idea. We've already added wfConfig as the wrapper for both Invariants (including orchestrator config) and things like scope. A further reworking of this stuff absolutely makes sense.

I absolutely agree. I think we have some cases where v2 is excessively lax. The specific thing you mention in the description has to do with how we process arguments. In v1, we started out with global keys, then added local keys in by trying to cut the function identity off of the key. In v2, we do the opposite: we start with a local key, then try (in a very permissive way) to add in the function identity; if that doesn't work, we're pretty lax. We could certainly tighten that up, and I imagine that we'd find other cases if we added these tests back in.

This is never triggered in unit tests. It is possible that recent fixes have also mitigated this. For now, I am going to add a log message. If we see the log message in the wild, we can go on to investigate; if not, I think we can just delete this code path.

This falls into the same bucket as https://phabricator.wikimedia.org/T423694 : content issue in a nested function is not being propagated correctly.

So, the problem here doesn't have anything to do with argument references per se. Here's what is happening.

In T407827#11749065, @Jdforrester-WMF wrote:

I'm afraid this still fails with v2. Shall we move out to re-triage?

In T419931#11775805, @99of9 wrote:

I don't think I have much insight here. If the answer is we're not allowed this behaviour anymore, I don't think that's a big deal. None of the current usage looks compelling.

Maybe close as not done then have a think more generally about how functions which return functions should/could work.

Z0 is already used for other kinds of magic, unfortunately. It's reserved for the frontend as a placeholder for a still-unallocated ZID. I would be open to us allocating a different ZID as the "anonymous callback" ZID, though!

Unassigning myself here :(

Re-assigning to @Jdforrester-WMF , who made the fix.

Now fixed in v2. Moving to ready to deploy.

This returns a Z516 error in both v1 and v2, as of HEAD. Moving to Ready to Deploy.