Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ement Taxy Room List: Unread category is filled with read rooms #97

Closed
MrRoy opened this issue Sep 16, 2022 · 13 comments
Closed

Ement Taxy Room List: Unread category is filled with read rooms #97

MrRoy opened this issue Sep 16, 2022 · 13 comments

Comments

@MrRoy
Copy link

MrRoy commented Sep 16, 2022

Hello,

In the ement-taxy-room-list, I have 128 rooms which are present in the unread category, despite actually being read. When I open them and mark them as read using the space bar, it closes the room but the room stays in the unread category. Opening another Matrix client doesn't show these rooms as unread. Also, the 'unread' column is empty for those rooms.
The Messages buffer doesn't show anything when using these rooms, or marking them as read
Not sure if it is relevant, but all of these rooms are rooms created from Matrix bridges (Facebook and Telegram mainly), though I do have bridged rooms that aren't stuck in the unread category.
swappy-20220915_202239

@alphapapa
Copy link
Owner

Hi Julien,

Thanks for reporting this.

Do those "unread" rooms have buffers opened for them? If so, please evaluate this function, refresh the room list, and see if they're still shown as unread:

(defun ement--room-unread-p (room session)
  "Return non-nil if ROOM is considered unread for SESSION.
The room is unread if it has a modified, live buffer; if it has
non-zero unread notification counts; or if its fully-read marker
is not at the latest known message event."
  ;; Roughly equivalent to the "red/gray/bold/idle" states listed in
  ;; <https://github.com/matrix-org/matrix-react-sdk/blob/b0af163002e8252d99b6d7075c83aadd91866735/docs/room-list-store.md#list-ordering-algorithm-importance>.
  (pcase-let* (((cl-struct ement-room timeline account-data unread-notifications receipts
                           (local (map buffer)))
                room)
               ((cl-struct ement-session user) session)
               ((cl-struct ement-user (id our-id)) user)
               ((map notification_count highlight_count) unread-notifications)
               (fully-read-event-id (map-nested-elt (alist-get "m.fully_read" account-data nil nil #'equal)
                                                    '(content event_id))))
    (or (and unread-notifications
             (or (not (zerop notification_count))
                 (not (zerop highlight_count))))
        ;; NOTE: This is *WAY* too complicated, but it seems roughly equivalent to doesRoomHaveUnreadMessages() from
        ;; <https://github.com/matrix-org/matrix-react-sdk/blob/7fa01ffb068f014506041bce5f02df4f17305f02/src/Unread.ts#L52>.
        (when timeline
          ;; A room should rarely, if ever, have a nil timeline, but in case it does
          ;; (which apparently can happen, given user reports), it should not be
          ;; considered unread.
          (cl-labels ((event-counts-toward-unread-p
                       (event) (not (member (ement-event-type event) '("m.room.member" "m.reaction")))))
            (let ((our-read-receipt-event-id (car (gethash our-id receipts)))
                  (first-counting-event (cl-find-if #'event-counts-toward-unread-p timeline)))
              (cond ((equal fully-read-event-id (ement-event-id (car timeline)))
                     ;; The fully-read marker is at the last known event: not unread.
                     nil)
                    ((and (not our-read-receipt-event-id)
                          (when first-counting-event
                            (and (not (equal fully-read-event-id (ement-event-id first-counting-event)))
                                 (not (equal our-id (ement-user-id (ement-event-sender first-counting-event)))))))
                     ;; A missing read-receipt failsafes to marking the
                     ;; room unread, unless the fully-read marker is at
                     ;; the latest counting event or we sent the latest
                     ;; counting event.
                     t)
                    ((not (equal our-id (ement-user-id (ement-event-sender (car timeline)))))
                     ;; If we sent the last event in the room, the room is not unread.
                     nil)
                    ((and first-counting-event
                          (equal our-id (ement-user-id (ement-event-sender first-counting-event))))
                     ;; If we sent the last counting event in the room,
                     ;; the room is not unread.
                     nil)
                    ((cl-loop for event in timeline
                              when (event-counts-toward-unread-p event)
                              return (and (not (equal our-read-receipt-event-id (ement-event-id event)))
                                          (not (equal fully-read-event-id (ement-event-id event)))))
                     t))))))))

@MrRoy
Copy link
Author

MrRoy commented Sep 16, 2022

Thanks for the prompt response
Most of these rooms don't have buffers opened for them, except the first few where I tried to to mark them as read.
I evaluated the function and killed the taxy room list & normal room list buffers, but when I reopened the taxy room list, I still had the same 128 "stuck" unread rooms.

@alphapapa
Copy link
Owner

Ok, please evaluate this function:

(defun test/ement--room-unread-p (room session)
  "Return non-nil if ROOM is considered unread for SESSION.
The room is unread if it has a modified, live buffer; if it has
non-zero unread notification counts; or if its fully-read marker
is not at the latest known message event."
  ;; Roughly equivalent to the "red/gray/bold/idle" states listed in
  ;; <https://github.com/matrix-org/matrix-react-sdk/blob/b0af163002e8252d99b6d7075c83aadd91866735/docs/room-list-store.md#list-ordering-algorithm-importance>.
  (pcase-let* (((cl-struct ement-room timeline account-data unread-notifications receipts
                           (local (map buffer)))
                room)
               ((cl-struct ement-session user) session)
               ((cl-struct ement-user (id our-id)) user)
               ((map notification_count highlight_count) unread-notifications)
               (fully-read-event-id (map-nested-elt (alist-get "m.fully_read" account-data nil nil #'equal)
                                                    '(content event_id))))
    (or (and unread-notifications
             (or (not (zerop notification_count))
                 (not (zerop highlight_count))))
        ;; NOTE: This is *WAY* too complicated, but it seems roughly equivalent to doesRoomHaveUnreadMessages() from
        ;; <https://github.com/matrix-org/matrix-react-sdk/blob/7fa01ffb068f014506041bce5f02df4f17305f02/src/Unread.ts#L52>.
        (when timeline
          ;; A room should rarely, if ever, have a nil timeline, but in case it does
          ;; (which apparently can happen, given user reports), it should not be
          ;; considered unread.
          (cl-labels ((event-counts-toward-unread-p
                       (event) (not (member (ement-event-type event) '("m.room.member" "m.reaction")))))
            (let ((our-read-receipt-event-id (car (gethash our-id receipts)))
                  (first-counting-event (cl-find-if #'event-counts-toward-unread-p timeline)))
              (cond ((equal fully-read-event-id (ement-event-id (car timeline)))
                     ;; The fully-read marker is at the last known event: not unread.
                     nil)
                    ((and (not our-read-receipt-event-id)
                          (when first-counting-event
                            (and (not (equal fully-read-event-id (ement-event-id first-counting-event)))
                                 (not (equal our-id (ement-user-id (ement-event-sender first-counting-event)))))))
                     (message "test/ement--room-unread-p: FAILSAFE FOR ROOM:%S" (ement-room-display-name room))
                     ;; A missing read-receipt failsafes to marking the
                     ;; room unread, unless the fully-read marker is at
                     ;; the latest counting event or we sent the latest
                     ;; counting event.
                     t)
                    ((not (equal our-id (ement-user-id (ement-event-sender (car timeline)))))
                     ;; If we sent the last event in the room, the room is not unread.
                     nil)
                    ((and first-counting-event
                          (equal our-id (ement-user-id (ement-event-sender first-counting-event))))
                     ;; If we sent the last counting event in the room,
                     ;; the room is not unread.
                     nil)
                    ((cl-loop for event in timeline
                              when (event-counts-toward-unread-p event)
                              return (and (not (equal our-read-receipt-event-id (ement-event-id event)))
                                          (not (equal fully-read-event-id (ement-event-id event)))))
                     (message "test/ement--room-unread-p: FOUND EVENT FOR ROOM:%S" (ement-room-display-name room))
                     t))))))))

Then switch to one of the incorrectly marked rooms' buffers, and do M-: (test/ement--room-unread-p) RET, and see what message is printed.

@MrRoy
Copy link
Author

MrRoy commented Sep 16, 2022

I get the following messages:

eval-expression: Wrong number of arguments: ((t) (room session) "Return non-nil if ROOM is considered unread for SESSION.
The room is unread if it has a modified, live buffer; if it has
non-zero unread notification counts; or if its fully-read marker
is not at the latest known message event." (progn (ignore (progn (and (memq (type-of room) cl-struct-ement-room-tags) t))) (let* ((x27 (aref room 6)) (x28 (aref room 8)) (x29 (aref room 9)) (x30 (aref room 19)) (x31 (aref room 18))) (progn (ignore (mapp x31)) (let* ((x32 (map-elt x31 'buffer))) (let ((timeline x27) (account-data x28) (unread-notifications x29) (receipts x30) (buffer x32)) (progn (ignore (progn (and ... t))) (let* ((x26 ...)) (let (...) (progn ... ...)))))))))), 0
Mark set

@alphapapa
Copy link
Owner

alphapapa commented Sep 16, 2022

Oops, sorry. Please do this: M-: (test/ement--room-unread-p ement-room ement-session) RET

@MrRoy
Copy link
Author

MrRoy commented Sep 16, 2022

I get this output:
test/ement--room-unread-p: FOUND EVENT FOR ROOM:"[removed] (Telegram)"

@alphapapa
Copy link
Owner

Ok, I added another debug output that will show the event being found, please eval this function and repeat the test:

;; This buffer is for text that is not saved, and for Lisp evaluation.
;; To create a file, visit it with C-x C-f and enter text in its buffer.

(defun test/ement--room-unread-p (room session)
  "Return non-nil if ROOM is considered unread for SESSION.
The room is unread if it has a modified, live buffer; if it has
non-zero unread notification counts; or if its fully-read marker
is not at the latest known message event."
  ;; Roughly equivalent to the "red/gray/bold/idle" states listed in
  ;; <https://github.com/matrix-org/matrix-react-sdk/blob/b0af163002e8252d99b6d7075c83aadd91866735/docs/room-list-store.md#list-ordering-algorithm-importance>.
  (pcase-let* (((cl-struct ement-room timeline account-data unread-notifications receipts
                           (local (map buffer)))
                room)
               ((cl-struct ement-session user) session)
               ((cl-struct ement-user (id our-id)) user)
               ((map notification_count highlight_count) unread-notifications)
               (fully-read-event-id (map-nested-elt (alist-get "m.fully_read" account-data nil nil #'equal)
                                                    '(content event_id))))
    (or (and unread-notifications
             (or (not (zerop notification_count))
                 (not (zerop highlight_count))))
        ;; NOTE: This is *WAY* too complicated, but it seems roughly equivalent to doesRoomHaveUnreadMessages() from
        ;; <https://github.com/matrix-org/matrix-react-sdk/blob/7fa01ffb068f014506041bce5f02df4f17305f02/src/Unread.ts#L52>.
        (when timeline
          ;; A room should rarely, if ever, have a nil timeline, but in case it does
          ;; (which apparently can happen, given user reports), it should not be
          ;; considered unread.
          (cl-labels ((event-counts-toward-unread-p
                       (event) (not (member (ement-event-type event) '("m.room.member" "m.reaction")))))
            (let ((our-read-receipt-event-id (car (gethash our-id receipts)))
                  (first-counting-event (cl-find-if #'event-counts-toward-unread-p timeline)))
              (cond ((equal fully-read-event-id (ement-event-id (car timeline)))
                     ;; The fully-read marker is at the last known event: not unread.
                     nil)
                    ((and (not our-read-receipt-event-id)
                          (when first-counting-event
                            (and (not (equal fully-read-event-id (ement-event-id first-counting-event)))
                                 (not (equal our-id (ement-user-id (ement-event-sender first-counting-event)))))))
                     (message "test/ement--room-unread-p: FAILSAFE FOR ROOM:%S" (ement-room-display-name room))
                     ;; A missing read-receipt failsafes to marking the
                     ;; room unread, unless the fully-read marker is at
                     ;; the latest counting event or we sent the latest
                     ;; counting event.
                     t)
                    ((not (equal our-id (ement-user-id (ement-event-sender (car timeline)))))
                     ;; If we sent the last event in the room, the room is not unread.
                     nil)
                    ((and first-counting-event
                          (equal our-id (ement-user-id (ement-event-sender first-counting-event))))
                     ;; If we sent the last counting event in the room,
                     ;; the room is not unread.
                     nil)
                    ((cl-loop for event in timeline
                              when (event-counts-toward-unread-p event)
                              do (message "test/ement--room-unread-p: FOUND EVENT FOR ROOM:%S  EVENT:%S" (ement-room-display-name room) event)
                              return (and (not (equal our-read-receipt-event-id (ement-event-id event)))
                                          (not (equal fully-read-event-id (ement-event-id event)))))
                     t))))))))

@MrRoy
Copy link
Author

MrRoy commented Sep 16, 2022

In the room I ran it before ([removed] (Telegram)) it just prints t, in the *Messages* buffer. I tried it on a couple more, and I noticed that some of these rooms give the following error when opening them up:

Searching for first unread event...
ement-room-retro-to: Assertion failed: (not (gethash event-id (ement-session-events session)))

Then, evaluating the command you sent me prints the following:
test/ement--room-unread-p: FAILSAFE FOR ROOM:"Facebook user (FB)"

@alphapapa
Copy link
Owner

Hm, this gets confusing. Please try this one:

(defun test/ement--room-unread-p (room session)
  "Return non-nil if ROOM is considered unread for SESSION.
The room is unread if it has a modified, live buffer; if it has
non-zero unread notification counts; or if its fully-read marker
is not at the latest known message event."
  ;; Roughly equivalent to the "red/gray/bold/idle" states listed in
  ;; <https://github.com/matrix-org/matrix-react-sdk/blob/b0af163002e8252d99b6d7075c83aadd91866735/docs/room-list-store.md#list-ordering-algorithm-importance>.
  (pcase-let* (((cl-struct ement-room timeline account-data unread-notifications receipts
                           (local (map buffer)))
                room)
               ((cl-struct ement-session user) session)
               ((cl-struct ement-user (id our-id)) user)
               ((map notification_count highlight_count) unread-notifications)
               (fully-read-event-id (map-nested-elt (alist-get "m.fully_read" account-data nil nil #'equal)
                                                    '(content event_id))))
    (or (and unread-notifications
             (or (not (zerop notification_count))
                 (not (zerop highlight_count))))
        ;; NOTE: This is *WAY* too complicated, but it seems roughly equivalent to doesRoomHaveUnreadMessages() from
        ;; <https://github.com/matrix-org/matrix-react-sdk/blob/7fa01ffb068f014506041bce5f02df4f17305f02/src/Unread.ts#L52>.
        (when timeline
          ;; A room should rarely, if ever, have a nil timeline, but in case it does
          ;; (which apparently can happen, given user reports), it should not be
          ;; considered unread.
          (cl-labels ((event-counts-toward-unread-p
                       (event) (not (member (ement-event-type event) '("m.room.member" "m.reaction")))))
            (let ((our-read-receipt-event-id (car (gethash our-id receipts)))
                  (first-counting-event (cl-find-if #'event-counts-toward-unread-p timeline)))
              (cond ((equal fully-read-event-id (ement-event-id (car timeline)))
                     ;; The fully-read marker is at the last known event: not unread.
                     nil)
                    ((and (not our-read-receipt-event-id)
                          (when first-counting-event
                            (and (not (equal fully-read-event-id (ement-event-id first-counting-event)))
                                 (not (equal our-id (ement-user-id (ement-event-sender first-counting-event)))))))
                     (message "test/ement--room-unread-p: FAILSAFE FOR ROOM:%S  OUR-READ-RECEIPT-EVENT-ID:%S  FIRST-COUNTING-EVENT:%S  FULLY-READ-EVENT-ID:%S  OUR-ID:%S"
                              (ement-room-display-name room) our-read-receipt-event-id first-counting-event fully-read-event-id our-id)
                     ;; A missing read-receipt failsafes to marking the
                     ;; room unread, unless the fully-read marker is at
                     ;; the latest counting event or we sent the latest
                     ;; counting event.
                     t)
                    ((not (equal our-id (ement-user-id (ement-event-sender (car timeline)))))
                     ;; If we sent the last event in the room, the room is not unread.
                     nil)
                    ((and first-counting-event
                          (equal our-id (ement-user-id (ement-event-sender first-counting-event))))
                     ;; If we sent the last counting event in the room,
                     ;; the room is not unread.
                     nil)
                    ((cl-loop for event in timeline
                              when (event-counts-toward-unread-p event)
                              do (message "test/ement--room-unread-p: FOUND EVENT FOR ROOM:%S  EVENT:%S" (ement-room-display-name room) event)
                              return (and (not (equal our-read-receipt-event-id (ement-event-id event)))
                                          (not (equal fully-read-event-id (ement-event-id event)))))
                     t))))))))

@MrRoy
Copy link
Author

MrRoy commented Sep 16, 2022

Here's the output for one of the rooms:

test/ement--room-unread-p: FAILSAFE FOR ROOM:"Facebook user (FB)"  OUR-READ-RECEIPT-EVENT-ID:nil  FIRST-COUNTING-EVENT:#s(ement-event "$h0GLUfh3pV0gDmAwi99LSQbJzjcY48IMZtkUuIa6k4k" #s(ement-user "@facebook_100002663095911:matrix.jroy.ca" nil nil nil nil nil nil nil) ((bridgebot . "@facebookbot:matrix.jroy.ca") (creator . "@facebook_100002663095911:matrix.jroy.ca") (protocol (id . "facebook") (displayname . "Facebook Messenger") (avatar_url . "mxc://maunium.net/ygtkteZsXnGJLJHRchUwYWak")) (channel (id . "100002663095911") (displayname) (avatar_url))) 1639737030142 "uk.half-shot.bridge" ((age . 23540006880)) "net.maunium.facebook://facebook/100002663095911" nil nil)  FULLY-READ-EVENT-ID:"$dRxppLehxneljNj86Ph10GsN3Ccez-sQVUyBr_03LWI"  OUR-ID:"@jroy:matrix.jroy.ca"

Let me know if you want me to try it on more rooms

@alphapapa
Copy link
Owner

Ok, please evaluate this function, refresh the room list, and see if the spuriously unread rooms are no longer unread:

(defun ement--room-unread-p (room session)
  "Return non-nil if ROOM is considered unread for SESSION.
The room is unread if it has a modified, live buffer; if it has
non-zero unread notification counts; or if its fully-read marker
is not at the latest known message event."
  ;; Roughly equivalent to the "red/gray/bold/idle" states listed in
  ;; <https://github.com/matrix-org/matrix-react-sdk/blob/b0af163002e8252d99b6d7075c83aadd91866735/docs/room-list-store.md#list-ordering-algorithm-importance>.
  (pcase-let* (((cl-struct ement-room timeline account-data unread-notifications receipts
                           (local (map buffer)))
                room)
               ((cl-struct ement-session user) session)
               ((cl-struct ement-user (id our-id)) user)
               ((map notification_count highlight_count) unread-notifications)
               (fully-read-event-id (map-nested-elt (alist-get "m.fully_read" account-data nil nil #'equal)
                                                    '(content event_id))))
    ;; MAYBE: Ignore whether the buffer is modified.  Since we have a better handle on how
    ;; Matrix does notifications/unreads/highlights, maybe that's not needed, and it would
    ;; be more consistent to ignore it.
    (or (and buffer (buffer-modified-p buffer))
        (and unread-notifications
             (or (not (zerop notification_count))
                 (not (zerop highlight_count))))
        ;; NOTE: This is *WAY* too complicated, but it seems roughly equivalent to doesRoomHaveUnreadMessages() from
        ;; <https://github.com/matrix-org/matrix-react-sdk/blob/7fa01ffb068f014506041bce5f02df4f17305f02/src/Unread.ts#L52>.
        (when timeline
          ;; A room should rarely, if ever, have a nil timeline, but in case it does
          ;; (which apparently can happen, given user reports), it should not be
          ;; considered unread.
          (cl-labels ((event-counts-toward-unread-p
                       (event) (equal "m.room.message" (ement-event-type event))))
            (let ((our-read-receipt-event-id (car (gethash our-id receipts)))
                  (first-counting-event (cl-find-if #'event-counts-toward-unread-p timeline)))
              (cond ((equal fully-read-event-id (ement-event-id (car timeline)))
                     ;; The fully-read marker is at the last known event: not unread.
                     nil)
                    ((and (not our-read-receipt-event-id)
                          (when first-counting-event
                            (and (not (equal fully-read-event-id (ement-event-id first-counting-event)))
                                 (not (equal our-id (ement-user-id (ement-event-sender first-counting-event)))))))
                     ;; A missing read-receipt failsafes to marking the
                     ;; room unread, unless the fully-read marker is at
                     ;; the latest counting event or we sent the latest
                     ;; counting event.
                     t)
                    ((not (equal our-id (ement-user-id (ement-event-sender (car timeline)))))
                     ;; If we sent the last event in the room, the room is not unread.
                     nil)
                    ((and first-counting-event
                          (equal our-id (ement-user-id (ement-event-sender first-counting-event))))
                     ;; If we sent the last counting event in the room,
                     ;; the room is not unread.
                     nil)
                    ((cl-loop for event in timeline
                              when (event-counts-toward-unread-p event)
                              return (and (not (equal our-read-receipt-event-id (ement-event-id event)))
                                          (not (equal fully-read-event-id (ement-event-id event)))))
                     t))))))))

@MrRoy
Copy link
Author

MrRoy commented Sep 16, 2022

Yes! Well, mostly. I'm down to 9 rooms now. Much better now :)
I ran the (test/ement--room-unread-p ement-room ement-session) command in the remaining rooms, but all I get in *Messages* is t.

@MrRoy
Copy link
Author

MrRoy commented Sep 16, 2022

Doing M-x ement-room-mark-read RET on those remaining rooms did mark them as read properly, so after that they left the Unread category.
So the fix from your last post solved the problem

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants