[BUG] The reduce operator and :reduce run prefix behave differently on empty input

[rmunn]

5.1.23 blocker, IMHO, though it shouldn't take too long to resolve.

Describe the bug
The reduce operator and :reduce prefix behave differently on empty input. One or the other should be adjusted so they behave the same. The operator will always produce output, even if its input is empty (the output is the value of the accumulator). The filter run prefix will produce an empty output if its input is empty, and will only produce output if there was at least one input item. This is because of the if(results.length > 0) check in the filter run prefix version, which is lacking in the operator version.

To Reproduce
Steps to reproduce the behavior:

1. Run the "nonexistent tag" example for the reduce operator
2. Run the "nonexistent tag" example for the :reduce filter run prefix
3. Note the different behavior with regard to the else operator (or :else prefix)

Expected behavior
Both the operator and the filter run prefix should have exactly the same behavior with regard to empty input. There are four ways I can think of to make this happen:

1. Make the operator return empty output when its input is empty, and don't change the way the filter run prefix acts.
2. Make the filter run prefix return the accumulator's initial value when its input is empty.
3. Make the filter run prefix return the accumulator's initial value when its input is empty, and additionally provide a way for filter run prefixes to take an extra argument the way operators do.
4. Entirely change the behavior of both reduce and :reduce: if no initial accumulator value given, use first list item for initial value. Use accumulator value only if given. (On empty input, return empty output, just like option 1).

Of those four, I strongly prefer the first one. I had to write extra documentation for reduce to make it plain that it will always produce output even when its input is empty, and although my reasons for that were based on the fact that that's how it works in functional programming, I'm re-evaluating that opinion. The fact that I had to write extra documentation just goes to show that it's non-intuitive for most people that empty input would produce non-empty output, and the right way for the reduce operator would have been for it to produce empty output when its input is empty, because that's what other operators do and that's what most people will expect. Thankfully, 5.1.23 isn't released yet so it's not too late to change this.

Option two is possible, but means that the :reduce prefix can never return, say, 0 when asked to sum up an empty list of numbers. And the current :reduce example (which uses :else[[0]] to specify a default value for empty input) would then be impossible.

Option three is a major syntax change, which I don't like so late in the release cycle. However, there's one good point in its favor. What would you expect to happen when you write 1 2 3 :reduce[multiply<accumulator>]? You'd expect to get 6, right? But instead you get 0, because the initial value of the accumulator is the empty string, and math operators treat the empty string as 0. To get what you actually expect from :reduce[multiply<accumulator>], you have to specify an initial accumulator value of 1. That's possible with the operator, but impossible with the filter run prefix!

Option four is not a major syntax change, but is a bit of a behavior change. But if we're going to do it, now's the time. Basically, right now reduce acts like the functional programming function called fold, not like the function called reduce! The rules for reduce in functional programming are "first item in list is initial accumulator value, and it's an error to pass in an empty list". The function where you specify an initial accumulator value and an input list, and the return value on empty input is the initial accumulator value, that one is called fold in functional programming. I do not suggest creating a fold operator, as most people will have no idea what it does! But I do think that it would be good for the reduce operator to be able to actually act like reduce, so that the multiply<accumulator> result will work the way people expect.

So I suggest that either we make the small change and let the reduce operator also return empty input (option one), or we take option four and let the reduce operator have two different behaviors, one for when no initial accumulator value is passed, and one for when an initial accumulator value is provided. Whichever one we do, we'd better do it soon, because right now there's an inconsistency that we don't want to allow into the release.

[rmunn]

Having thought about it some more, option 4 is probably not viable. Sure, it's fine to say "Use the first value in the list as the initial accumulator value" when the operation you're doing acts directly on the list contents, such as in the 1 2 3 :reduce[multiply<accumulator>] example. But what if the operation is looking something up, such as a price field? Then you can't just do oranges apples :reduce[get[price]multiply{!!quantity}add<accumulator>]. Because you're needing the first list item, oranges, to go through the same transformation (get price field, multiply by quantity field) as the other items. So you have to start with an initial accumulator value.

In other words, we can't implement the functional-programming function called reduce. We have to implement fold.

Another thought: option 3 would require adding the possibility of filter run prefix suffixes, the very thing I argued against in #5166 (comment). But because of the multiply<accumulator> example, they might be necessary if the :reduce prefix is to exist. We might have to allow writing =1 =2 =3 :reduce:1[multiply<accumulator>], where the second : marks the start of the prefix suffix (and yikes, I don't like that). Either that, or make a note in the filter run prefix examples that multiply<> is a bit of a special case and you have to do something like :reduce[<index>compare:number:gt[0]then<currentTiddler>multiply<accumulator>else<currentTiddler>]. Which is super ugly but the only way to make the multiply example work correctly. Double yikes. After writing that out, suddenly I'm much more open to the idea of filter run prefix suffixes, as ugly as that phrase is to think about.

[rmunn]

Alternate possibility: let filter run prefixes take a second parameter the way operators do, so that instead of =1 =2 =3 :reduce:1[multiply<accumulator>], it would be =1 =2 =3 :reduce[multiply<accumulator>],[1]. I'm pretty sure the : syntax (filter run prefix suffixes) would be able to be parsed in the parseFilter function of core/modules/filters.js, but would the latter syntax (the ,[1] syntax that would also allow for things like ,<defaultValue>) be parseable? I'm not sure. But it's something to consider.

[saqimtiaz]

@rmunn I pondered over this difference when writing the :reduce prefix. I didn't feel at liberty to change the behaviour of the reduce[] operator and since prefixes cannot accept additional arguments, I settled for trying to make sure the examples illustrated the difference. I had not considered the multiply scenario.

Greater consistency is always preferable if possible. Personally I would favour #1 or #4 of the proposed solutions, since I assume #3 is not an option based on previous discussions.

Now it could be that some functionality just cannot be implemented with filter run prefixes. However even if we were to decide against the :reduce prefix, this issue highlights a shortcoming in both the behaviour of the reduce[] operator and the flexibility of the prefixes. I do feel like the reduce operator needs to return an empty output on empty input and at the very least should be reconsidered.

Thoughts @Jermolene ?

I have a packed schedule today, but I'll try to follow the discussion as and when I can.

[Jermolene]

Just to focus on one part of this: oddly, I don't have much problem with the complexity of the multiply example:

:reduce[<index>compare:number:gt[0]then<currentTiddler>multiply<accumulator>else<currentTiddler>]

My reasoning is that the rationale for the reduce operator is to give us filters that hugely expand what is possible. It's also pretty complicated conceptually, the kind of thing that learners would find hard to understand until they reach the point where they've experienced the dead-ends that it is intended to resolve.

We mitigate the complexity by making the operator closely resemble the traditional way that the functionality is exposed in languages like JavaScript, albeit mapped through to TiddlyWiki idioms.

That gives us undeniably verbose examples like the one above, but the result is very readable. The problem with introducing shortcut syntaxes is that the result will be more cryptic, requiring knowledge of slightly more special rules in order to understand it.

Other possibilities to improve the readability of the example above might be:

• Allowing line breaks in filter strings (the problem is actually currently that attributes can't contain line breaks, filter strings are OK with them)
• Allowing whitespace into filter expressions for reability
• Allowing comments in filter expressions
• @saqimtiaz's suggestion in [IDEA] TiddlyWiki filter visualizer/helper tool #5058 of a filter expression visualiser might be helpful, particularly if integrated into the editor

[rmunn]

I'm currently busy writing the demo I'm going to use to present to my colleagues so I don't have time to implement this today, but I'll be happy to implement option 1 starting tomorrow if nobody else gets to it first. I'd prefer option 4, but as I mentioned earlier it's not really viable. There's really no way to make our reduce operator take the first value of the input as the initial accumulator value; it only works for simple examples but falls apart whenever you try to do anything at all complex with it. So after having considered that, I've concluded that option 1 is the way to go. We make the reduce operator return empty output when it gets empty input, and then we can remove the documentation (and examples) that say "Beware of how this interacts with the else operator", because the interaction with else is going to be what people expect it to be. And if they really want a "0" result from adding an empty list, which reduce[add<accumulator>],[0] will no longer give them once that change goes in... then it's as simple as reduce[add<accumulator]else[0].

So I'm 👍 on option 1 now.

[saqimtiaz]

@rmunn I wont be able to work on this before the weekend at the earliest, so please feel free tomorrow. We should add the multiply example to the docs as well.

As for the filter visualizer @Jermolene, I think we can get that ready for 5.1.24. I have some thoughts and leads to discuss once 5.1.23 is released and hopefully things calm down a bit on my end, the volunteering is tying up a fair bit of time at the moment.

[Jermolene]

By the way, I support option 1.

[rmunn]

PR: #5255

[rmunn]

I've chosen to go with option 1 for the PR, as this brings reduce and :reduce into feature parity (except that initial accumulator values can't be specified with :reduce because it lacks the syntax for it). I've also updated the documentation, and mentioned the multiply<accumulator> gotcha where initial accumulator values are necessary to get the result most people would expect. (And if in the future we somehow figure out a syntax we like for allowing a second parameter to filter run prefixes, then we can remove the warning in the :reduce prefix documentation about the multiply<accumulator> case.)

[saqimtiaz]

@rmunn thank you.

[Jermolene]

Great, thanks @rmunn @saqimtiaz