Points already awarded to ancestor not working correctly

[PixelOrange]

See here: http://www.reddit.com/r/PixelOrange/comments/1kp3uz/test_cmv/cd02ezg

https://github.com/alexames/DeltaBot/blob/master/deltabot/deltabot.py#L252 does not appear to be working properly. It allowed me to make several delta additions to the same user.

[Chillee]

I'm somewhat confused about why that function does what it does. Is it checking whether the author had already gave the parent comment's author a delta in the same tree?

Still, I think I know what the error is.

[bryantp]

Just to confirm. This is not what you wanted right.

http://www.reddit.com/r/botTestingYDD/comments/1urspm/this_is_a_test/cel2ahr

[PixelOrange]

This comment is what should happen after the first response: http://www.reddit.com/r/botTestingYDD/comments/1urspm/this_is_a_test/cel4qp0

[Chillee]

Apparently that should be allowed. The way the function is worded, it seems to only want it not to happen in each branch of the tree. Since those are all separate branches, it seems that it would be intended behavior.

Assuming that you wanted to remove all deltas within an entire tree, it would take a much bigger overhaul of the points_already_awarded_to_ancestor function. It would probably require a full tranversal of the comment tree.

However, another approach that comes to mind would be limiting the OP to one delta per user per thread.

Note: I'm not sure what the mods want as the intended behavior. I saw the code, and fixed it so that it worked as the code was intended to work.

PixelOrange, your link links to nothing.

[PixelOrange]

Fixed my link, that was weird.

Okay, so TestingAcctount12345 is responding to TestingAccount3 multiple times. That's not something that should be allowed. It should say, "You've already given the user a delta."

But it ALSO should not allow for multiple deltas in the same tree.

Basically, the only time that you should be able to award two deltas is if they are not at all linked by the same root comment.

I think that it would make more sense, personally, to only allow one delta per awarder per awardee per thread. It's rare that the awarder would want to give the awardee more than one delta anyway.

[Chillee]

So one delta from a user per top level comment? That would require a DFS. Personally, I think a simpler, and better, approach would be to allow one delta per awarder per awardee per thread. As it is right now, a user would need to award the same person once in one comment tree, and once in another.

[Snorrrlax]

This whole situation has been hard for me to convey ever since it started, but here's what I think:

We decided that user A can only award user B one delta per comment tree. This was to make it more fair and less dependent on how 'delta happy' the awarder is. So originally the code was written as user A can only award user B one delta in the entire thread. This worked fine for a while until I noticed a couple of situations where this chain of events happened:

1. User A awarded user B a delta in a comment thread for changing their view on a particular aspect.
2. User C created a new comment thread about a separate part of the view.
3. User A replied to user C with further questions, to which user B replied, causing user A to change their view on this different part.
4. User A then awarded user B a delta, which DeltaBot ignored.
5. They contacted me about this and after thinking about it I decided that I wanted to allow this type of situation, where user A has awarded two deltas to user B in the post.

[Chillee]

Like this is what I thought wanted to be stopped.

http://www.reddit.com/r/changemyview/comments/1ul993/i_do_not_believe_that_pleasing_shareholders/cejftcr

[Snorrrlax]

That's correct as it is, because user A is trying to award user B two deltas in the same comment thread.

[bryantp]

Just to clarify. A root comment cannot be given multiple deltas by the same user.

(User A)Root Comment User   (User B)Child Comment with Delta   (User B)Another Child with Delta //This will not count.

However, for each tree, the root can be given a delta

Sub trees can have their roots given a delta.

(User A)Root Comment User   (User B)Child Comment     (User C)Child Comment with Delta //User B receives a delta   (User C)Child Comment with Delta //User A receives a delta

Again, if the same user tries to give a second delta, it is disallowed. If this is the case, wouldn't you just have to search the first level of comments? When the bot scans the new comments and finds a delta:

1) Find out the parent's id 2) Scan the first level of comments to see if a user has received a delta from the same user before. 3) If so, don't give the delta. If not, give the delta.

For things like deleted or edited comments, you should go down one more level to see if the bot has already approved the comment for a delta.

Looks like you guys commented while I was typing this.

So, a user can only give another user a Delta for an entire tree?

(User A)Root Comment User   (User B)Child Comment     (User C)Child Comment with Delta //User B receives a delta       (User B)Child Comment         (User C)Child Comment with Delta //Already gave user B a delta in this tree

[PixelOrange]

http://www.reddit.com/r/changemyview/comments/1xybos/it_is_impossible_for_there_to_be_a_homosexual/cffr0wx

This bug is still an issue.

[dubslow]

As currently written in the code, the ancestor function checks only the following:

If, in any parent of the candidate_awardee comment, the OP awards a delta, then this candidate is rejected.

However, this does not exclude the OP from awarding extra deltas to the candidate, as in the most recent comment, nor does it exclude the OP from awarding a delta to a sibling/cousin of the candidate (that is, the OP can also award a delta to comments with the same root as the candidate, but which do not have the candidate as a (grand)child).

Clearly the first case should be excluded; Snorrrlax, do you intend the second case to be excluded as well? (The third case, the OP awarding a delta to a comment with a different root from the candidate, is both allowed and clearly stated as allowed by Snorrrlax above.)

(Since it's not obvious, on reddit I am /u/Bunslow)

[Snorrrlax]

that is, the OP can also award a delta to comments with the same root as the candidate, but which do not have the candidate as a (grand)child

Can you explain this further? It's probably my fault but I don't really understand what you're asking.

[dubslow]

User A replies to OP (so this is a root comment);
        User B replies to A;
                OP awards B a delta
        User C replies to A;
                User B replies to C;
                        OP awards B a delta;

This is the case I was referring to. In this case, User B's comments are not directly related to each other (neither is a (grand)child of the other), but are cousins/siblings in the same root-comment-tree (in this specific situation, they are aunt and niece, lol). Should this be allowed?

[Snorrrlax]

Ah, I get you now. I suppose it comes down to whether or not a comment tree is likely to be talking about similar parts to the view, whereas a separate comment tree might be tackling a separate one. Therefore awarding a delta to the same user in two separate comment trees seems okay, in my opinion, as I explained above. Whether this can occur within the same comment tree, down a separate branch, is something I hadn't really thought about. But I think we could probably allow that too.

The main thing in all of this we wanted to disallow was:

User A makes a top level comment
    OP replies to user A, and at some point in an exchange between the two, awards a delta.
        User A replies again after this.
            OP awards another delta.

Maybe every other situation where OP awards user A another delta could be allowed? I'll ask the other mods.

What's your opinion on this?

[howbigis1gb]

I think the current status quo seems fine - especially since the wiki tracks the circumstances regarding the delta.

Unless the delta wasn't meant for User A, then that could be an issue - but I am not sure that it is an issue with the programming as much as it is with user responsibility.

[PixelOrange]

I'm still of the opinion that person A should only be allowed to give person B one delta per thread. The amount of instances in which anyone gives a delta to anyone else more than once per thread is already extremely low. I see no reason to make it even more complicated than this.

[chrisuehlinger]

The amount of instances in which anyone gives a delta to anyone else more than once per thread is already extremely low. I see no reason to make it even more complicated than this.

Keeping a running (and perpetually up to date) tally of the number of times "someone does something in a thread" is a task reddit makes pretty difficult. If this isn't something that happens very often, you might want to consider scrapping it. The potential for bugs as threads get bigger/older (and if people "do stuff" on old threads) is pretty big.

[Snorrrlax]

Okay, maybe we'll reevaluate this in the future then. Is the bug fixed that PixelOrange mentioned?

[dubslow]

See recent pull request for probable fix to all bugs.

From one of the commit logs:

This adds Python 3 support, as well as completely overhauling the
"check ancestor for points awarded code".

Instead of what(ever) it did previously, now this does the following:
For any potential point awarded,
  1) find the root of that whole comment tree
  2) search all comments in that tree for a DeltaBot confirmation message
        to the candidate awardee
If such a previous confirmation message is found, then disallow the re-award,
otherwise allow it. This is much simpler and much more thorough (I think).

Though it passed runtests.sh, I highly suggest PixelOrange (or someone else) test this more thoroughly to see if it actually fixes these bugs.

[Snorrrlax]

Thanks!

As for the User A/User B thing, some of us are a bit confused as to how it currently works, which makes for confusion when people say "I think we should keep it how it is/change it" etc. I feel like I used to know but I've forgotten.

[PixelOrange]

Closed due to issue #58