955645 - Use a conversation's statusType for ranking

Status: RESOLVED FIXED

(Instantbird Graveyard :: Conversation, defect)

Description

[(inactive) Nihanth Subramanya [:nhnt11]]

*** Original post on bio 2200 at 2013-10-03 22:19:00 UTC ***

Currently, convs that have a score are sorted first by statusType, then by score. Rather than do this, we need a way to incorporate the statusType in the scoring. The reasoning is that sometimes, a contact may be talked to often enough that we want them to show up near the top of the list even if they're away or mobile (or even offline).

Related: Chats are currently given STATUS_AVAILABLE, we may want to tweak this.

Comment 1

[aleth [:aleth]]

*** Original post on bio 2200 at 2013-10-09 09:08:06 UTC ***

Iirc there is a flag that tells you whether an account allows messages to offline contacts. If this is reliable, you could give offline contacts on such accounts a higher weight.

Chats from LIST or the logs are status=available, but we should generally list contacts first I think, i.e. the status for MUCs should be weighted differently.

Comment 2

[Florian Quèze [:florian]]

*** Original post on bio 2200 at 2013-10-09 09:14:20 UTC ***

(In reply to comment #1)
> Iirc there is a flag that tells you whether an account allows messages to
> offline contacts.

Don't go as far as the account: each account buddy has a canSendMessage flag (http://lxr.instantbird.org/instantbird/source/chat/components/public/imIStatusInfo.idl#43).

Comment 3

[(inactive) Nihanth Subramanya [:nhnt11]]

Attachment: Patch

*** Original post on bio 2200 as attmnt 2933 at 2013-10-09 21:01:00 UTC ***

This applies a bias to contacts based on their availability. If the statusType value is less than half the max status (Available), this is a negative bias. If it's greater than half, a positive bias is applied.

It also applies a negative bias to existing MUCs.

I'm seeing better results in the awesometab with these changes. Let me know what you think :)

Comment 4

[aleth [:aleth]]

Comment on attachment 8354708 [details] [diff] [review]
Patch

*** Original change on bio 2200 attmnt 2933 at 2013-10-10 19:14:44 UTC ***

>     let scoreA = aPossibleConvA.computedScore;
>     let scoreB = aPossibleConvB.computedScore;
>     // We want conversations with stats (both contacts and chats) to appear first,
>     // followed by contacts with no stats, and finally chats with no stats.
>     // Conversations with stats have a positive score.
>     // Contacts with no stats get a 0, and chats get -1.
>     let sign = function(x) x > 0 ? 1 : x < 0 ? -1 : 0;
>     return sign(scoreB) - sign(scoreA) ||
>+      scoreB - scoreA ||
>       aPossibleConvB.statusType - aPossibleConvA.statusType ||
>-      scoreB - scoreA ||
>       aPossibleConvA.lowerCaseName.localeCompare(aPossibleConvB.lowerCaseName);
>   },

Please update the comment when you change the logic ;)

This change privileges ranking over statusType, because the ranking now depends on the statusType. But it doesn't seem sensible to put highly ranked offline contacts up top if I can't talk to them. Please use comment 2 for this!

Could you take a look at the rank of your most frequent channels and your most frequent contacts and compare the score. My suspicion is that it's very hard for any contact to compete with a channel as channels are much noisier. This may require a different weight on channels as compared to contacts, what do you think? 

>@@ -628,17 +625,20 @@ PossibleConvFromContact.prototype = {
>     let stats = new ConversationStats();
>     for (let id of this.buddyIds) {
>       let buddyStats = gStatsByConvId[id];
>       if (buddyStats)
>         stats = stats.mergeWith(buddyStats);
>     }
>     if (gStatsByContactId)
>       gStatsByContactId[id] = stats;
>-    return stats.computedScore;
>+    let score = stats.computedScore;
>+    let maxStatus = Ci.imIStatusInfo.STATUS_AVAILABLE;
>+    score += score * (this.statusType - maxStatus / 2) / maxStatus;
>+    return score;
>   },
>   createConversation: function() this.contact.createConversation()
> };

Comment please.

Other than that, it's hard to judge these changes without trying them ;)

Comment 5

[Benedikt Pfeifer]

Comment on attachment 8354708 [details] [diff] [review]
Patch

*** Original change on bio 2200 attmnt 2933 at 2013-10-10 19:50:44 UTC ***

Only some comments on the code, none on the logic.

>diff --git a/instantbird/components/ibConvStatsService.js b/instantbird/components/ibConvStatsService.js
>@@ -628,17 +625,20 @@ PossibleConvFromContact.prototype = {
>     if (gStatsByContactId)
>       gStatsByContactId[id] = stats;
>-    return stats.computedScore;
>+    let score = stats.computedScore;
>+    let maxStatus = Ci.imIStatusInfo.STATUS_AVAILABLE;
>+    score += score * (this.statusType - maxStatus / 2) / maxStatus;

This will need a comment, explaining that some statuses get an increase and some a decrease of their score.
It can be considerably shortened too:
      score *= 0.5 + this.statusType/maxStatus;

>+  get computedScore() {
>+    let score;
>+    let stats = gStatsByConvId[this.id];
>+    if (stats) {

if (!stats)
  return this.isChat ? -1 : 0;

>+      score = stats.computedScore;
>+      // Give existing chats a negative bias. It's unlikely the user wants to
>+      // reopen them.
>+      if (this.isChat)
>+        score *= 0.8;
>+      return score;
>+    }

The indentation can be reduced because of the early return on "if(!stats)" now.

Comment 6

[(inactive) Nihanth Subramanya [:nhnt11]]

Attachment: Patch 2

*** Original post on bio 2200 as attmnt 2936 at 2013-10-10 20:54:00 UTC ***

>>     // We want conversations with stats (both contacts and chats) to appear first,
>>     // followed by contacts with no stats, and finally chats with no stats.
>>     // Conversations with stats have a positive score.
>>     // Contacts with no stats get a 0, and chats get -1.
>
>Please update the comment when you change the logic ;)

I don't think this comment needs to be updated.

Comment 7

[(inactive) Nihanth Subramanya [:nhnt11]]

Comment on attachment 8354708 [details] [diff] [review]
Patch

*** Original change on bio 2200 attmnt 2933 at 2013-10-10 20:54:05 UTC was without comment, so any subsequent comment numbers will be shifted ***

Comment 8

[aleth [:aleth]]

Comment on attachment 8354712 [details] [diff] [review]
Patch 2

*** Original change on bio 2200 attmnt 2936 at 2013-10-10 20:58:20 UTC ***

I think we should just try this in nightlies.

Comment 9

[Benedikt Pfeifer]

Comment on attachment 8354712 [details] [diff] [review]
Patch 2

*** Original change on bio 2200 attmnt 2936 at 2013-10-10 22:10:02 UTC was without comment, so any subsequent comment numbers will be shifted ***

Comment 10

[Patrick Cloke [:clokep]]

*** Original post on bio 2200 at 2013-10-11 01:37:49 UTC ***

http://hg.instantbird.org/instantbird/rev/59e18294a19d