model: Introduce data structures for "recent senders criterion" of user-mention autocomplete #692

sm-sayedi · 2024-05-20T11:34:19Z

This is the second PR in the series of PRs #608 is divided into, coming after #693. This solely focuses on bringing the data structures for the upcoming "recent senders" criterion of @-mention autocomplete. Next PR in the series: #828.

Fixes part of: #228

sm-sayedi · 2024-05-20T11:43:00Z

lib/model/message_list.dart

      if (_messageVisible(message)) {
        _addMessage(message);
+        store.recentSenders.handleMessage(message);
      }


Reply to your 608 comment:

Good question.

What does the dartdoc on that _messageVisible method say?

Based on that, what would be the consequence of having that condition control whether to make this call? And what would be the consequence of ignoring that condition when making this call?

I concluded that the call should be inside the if condition. We track the messages that are shown in the feed, and if they're not shown, then there's no need to show the users who sent these messages, in the autocomplete results. Thanks for the clue!!

If we look at web impl, recent_senders.process_stream_message in process_new_message doesn't seem to care about topic visibility policy. Not sure if that's intended or a bug.

Yeah, good comparison.

Here, _messageVisible says whether the message should appear in this particular message list:

/// Whether [message] should actually appear in this message list, /// given that it does belong to the narrow. /// /// This depends in particular on whether the message is muted in /// one way or another. /// /// See also [_allMessagesVisible]. bool _messageVisible(Message message) {

But if the message doesn't appear in this message list, it might still appear in a different one later. A message hidden from the combined feed because the stream is muted will appear if you navigate to that stream; a message hidden because the stream and/or topic is muted will appear if you navigate to the topic.

The recent-senders data structure isn't specific to a particular message list; it's shared across them all (within an account). So let's have the data it gets not be controlled by the message list's _messageVisible.

gnprice · 2024-05-22T19:23:22Z

Let's start applying the "buddy review" system on this PR, now that the start of the official GSoC period is imminent and it seems like everyone's actively engaged again.

@rajveermalviya, would you do the first review on this PR? Some quick bits of context to note:

The first few commits are part of autocomplete: In user-mention autocomplete results give priority to users in DM conversations #693, as mentioned in the description, and are getting reviewed there. So please focus on the commits that come after those.
The last round of review on the predecessor PR started here (and came as several separate comments): autocomplete: Sort user-mention autocomplete results #608 (review)

Some of that is about what's now in autocomplete: In user-mention autocomplete results give priority to users in DM conversations #693; some of it is about what's added in this PR.

Meanwhile we'll proceed with maintainer review for the first PR in this series, #693, because after the previous rounds of review (on #608) that one is further along.

As a reminder since we're just starting to use this system: when you get a buddy review request, please prioritize it right after important bugs and your regressions, ahead of most of your own PRs. (In particular I don't think anyone is currently working on an issue that would come ahead of doing buddy reviews.)

rajveermalviya

Some comments, otherwise looks great!
Thanks!

rajveermalviya · 2024-05-23T09:34:13Z

lib/model/recent_senders.dart

+
+  /// Extracts and keeps track of the necessary data from a [message] only
+  /// if it is a stream message.
+  void handleMessage(Message message) {


Suggested change

void handleMessage(Message message) {

void handleMessageEvent(MessageEvent event) {

final message = event.message;

Matches RecentDmConversationsView.handleMessageEvent.

This change would match it with other somewhat similar parts of the codebase, but there are places this method is called where there is no MessageEvent available, such as in model/message_list.dart file. In contrast, RecentDmConversationsView.handleMessageEvent is only called in PerAccountStore.handleEvent where the MessageEvent is available.

That makes sense.

rajveermalviya · 2024-05-23T09:35:40Z

lib/model/recent_senders.dart

+  /// Whether stream senders and topic senders are both not empty.
+  @visibleForTesting
+  bool get debugIsNotEmpty => _streamSenders.isNotEmpty && _topicSenders.isNotEmpty;


Suggested change

/// Whether stream senders and topic senders are both not empty.

@visibleForTesting

bool get debugIsNotEmpty => _streamSenders.isNotEmpty && _topicSenders.isNotEmpty;

remove, it's not used.

rajveermalviya · 2024-05-23T11:28:41Z

lib/model/message_list.dart

      if (_messageVisible(message)) {
        _addMessage(message);
+        store.recentSenders.handleMessage(message);
      }


If we look at web impl, recent_senders.process_stream_message in process_new_message doesn't seem to care about topic visibility policy. Not sure if that's intended or a bug.

sm-sayedi · 2024-05-29T15:48:43Z

Thanks, @rajveermalviya for the review! Pushed the revision. Please have a look.

sm-sayedi · 2024-05-29T16:07:35Z

test/model/autocomplete_test.dart

+    // test('fetching older messages', () async {
+    //   final users = generateUsers(count: 1500);
+    //   final store = eg.store();
+    //   final stream = eg.stream();
+    //   final narrow = StreamNarrow(stream.streamId);
+    //   final subscription = eg.subscription(stream);
+    //   await store.addStream(stream);
+    //   await store.addSubscription(subscription);


@gnprice I have commented out this test for now as it wouldn't pass. I spent a significant amount of time to diagnose what was the problem, but unfortunately, I wasn't able to figure it out. I would appreciate it if you can have a look at it.

Yeah. I think fundamentally the issue here is that these tests using await Future(() {}); are fragile.

They're trying to make several different things happen in a certain order, but those things are asynchronous in the code. The await Future(() {}) basically allows each thread of async control flow to advance from one await to the next await… but what that means in terms of actual behavior of the models depends in a very brittle way on the exact details of how the code being tested is written. Small differences in that code will cause things in the test to happen in a different order and break the test.

That's also why the existing tests of this flavor have so many checks like check(done).isFalse() — those are there to make sure that the test's assumptions about what order things happen in are satisfied. That way, if those assumptions are broken by a refactor or by a change to the test, the test fails instead of silently ceasing to successfully test what it's meant to test.

It looks like this test is failing at the check(done).isFalse() here:

await act(store); await Future(() {}); check(done).isFalse(); await Future(() {}); check(done).isTrue(); expect(view.results);

so IOW the search is done sooner than the test expects. Presumably it's due to some difference between the fetchOlder in this test, and the variations on handleEvent found in the neighboring tests.

The test passes if you make this change:

@@ -293,7 +296,6 @@ void main() { view.addListener(() { done = true; }); view.query = MentionAutocompleteQuery(rawQuery); - await Future(() {}); check(done).isFalse(); await act(store);

But in that case is the test still successfully testing what it's supposed to test? I'm not sure what the assumptions are of your doCheck method.

(Probably a chat thread would be a good place to go into further detail.)

rajveermalviya

Apart from that test, it looks great :)
Thanks!

hackerkid

I didn't do any review of the code. But approving since this seems to be part of Zulip's this year's GSOC process.

gnprice · 2024-06-04T02:06:40Z

@hackerkid Putting this back in your queue after we discussed today how mentor review is meant to work 🙂

(#713 would be a good PR to review first, though — it's smaller, and also this one is stacked atop #693 which is still in progress, so there's no rush for this one to get through review before #693 is ready.)

sm-sayedi · 2024-06-11T19:12:13Z

@hackerkid Now that #693 is merged, this PR is now ready for the mentor review!

hackerkid · 2024-06-19T20:52:28Z

lib/model/recent_senders.dart

+  // TODO: remove
+
+  /// The maximum id in the tracker list.
+  ///
+  /// Returns -1 if the tracker list is empty.


Are these comments still relevant?

Yeah, I think they're relevant, specifically the "Returns null if the tracker list is empty" part which will help the reader what to expect.

sm-sayedi · 2024-07-19T20:18:08Z

Thanks @gnprice for the review. Updates pushed. Please have a look.

sm-sayedi · 2024-07-19T20:23:04Z

lib/model/recent_senders.dart

+  }
+}
+
+class MessageIdTracker {


Making this class private would cause the following lint message, thus failing the CI:

Invalid use of a private type in a public API. Try making the private type public, or making the API that uses the private type also be private.

Yeah, because the type appears in RecentSenders.streamSenders and RecentSenders.topicSenders.

Still we can keep a @visibleForTesting annotation on this, right? It looks like that went away in the latest revision.

gnprice

Thanks for the revision! This is very close now. Comments below.

For the remaining comments on the tests, I decided it'd be clearest to demonstrate rather than describe, so I'll push to this branch some added commits that make those changes. Then please leave those in the branch, after your main commits, while acting on the several other comments below. For the two comments that request followup commits, those can go after my test commits.

gnprice · 2024-07-19T21:34:03Z

lib/model/recent_senders.dart

+  /// [newIds] should be sorted ascendingly.
+  void addAll(List<int> newIds) {
+    if (ids.isEmpty) {
+      ids = QueueList.from(newIds);


This doesn't improve over calling addAll (following up on #692 (comment) ), because it's still copying the list.

This isn't any worse than that either, though, and I guess it makes it more parallel with the setUnion case below — now in both cases we copy the input (and in one case the existing data) into a new QueueList.

To do better would require having this method take a QueueList so that it can actually say ids = newIds;, and having that input list be constructed as a QueueList in the first place. If you'd like to try doing that, let's leave it as a separate followup commit, in the interest of letting the rest of these changes converge toward merging.

gnprice · 2024-07-19T21:37:01Z

lib/model/recent_senders.dart

+
+  /// Add the message ID to the tracker list at the proper place, if not present.
+  ///
+  /// Optimized, taking O(log n) time for the case where that place is the end,


This is accurate now, but I actually did want the O(1) behavior 🙂 (following up on #692 (comment) ).

That can be a followup commit, though, again to help the other changes converge toward merging. I might squash it in, or I might leave it out and make that optimization myself.

gnprice · 2024-07-19T21:42:31Z

lib/model/recent_senders.dart

+      (messagesByUserInStream[(streamId, senderId)] ??= []).add(id);
+      (messagesByUserInTopic[(streamId, topic, senderId)] ??= []).add(id);


This function is simplified slightly in this revision, but it can be simplified a lot more. There's only one channel/stream ID and one topic in the whole method. So in particular:

Suggested change

(messagesByUserInStream[(streamId, senderId)] ??= []).add(id);

(messagesByUserInTopic[(streamId, topic, senderId)] ??= []).add(id);

(messagesByUser[senderId] ??= []).add(id);

gnprice · 2024-07-19T21:44:00Z

lib/model/recent_senders.dart

+    if (event.messageType != MessageType.stream) return;
+    final messagesByUserInStream = <(int, int), List<int>>{};
+    final messagesByUserInTopic = <(int, String, int), List<int>>{};
+
+    final DeleteMessageEvent(:streamId!, :topic!) = event;
+    for (final id in event.messageIds) {


nit: group closely-related lines into stanzas:

Suggested change

if (event.messageType != MessageType.stream) return;

final messagesByUserInStream = <(int, int), List<int>>{};

final messagesByUserInTopic = <(int, String, int), List<int>>{};

final DeleteMessageEvent(:streamId!, :topic!) = event;

for (final id in event.messageIds) {

if (event.messageType != MessageType.stream) return;

final DeleteMessageEvent(:streamId!, :topic!) = event;

final messagesByUserInStream = <(int, int), List<int>>{};

final messagesByUserInTopic = <(int, String, int), List<int>>{};

for (final id in event.messageIds) {

gnprice · 2024-07-19T21:50:52Z

lib/model/recent_senders.dart

+  }
+}
+
+class MessageIdTracker {


Yeah, because the type appears in RecentSenders.streamSenders and RecentSenders.topicSenders.

Still we can keep a @visibleForTesting annotation on this, right? It looks like that went away in the latest revision.

gnprice · 2024-07-19T22:11:34Z

test/model/recent_senders_test.dart

+          eg.streamMessage(stream: stream1, topic: 'a', sender: user10, id: 200),
+        ];
+        setupModel(existingMessages);
+
+        final dmMessage = eg.dmMessage(from: eg.otherUser, to: [eg.selfUser], id: 200);


bad test data: message ID collision

(and can't be the same message seen twice, because stream/channel messages can't turn into DMs or vice versa)

gnprice · 2024-07-19T22:12:56Z

test/model/recent_senders_test.dart

+    group('single tracker', () {
+      test('batch goes before the existing messages', () {
+        final existingMessages = [
+          eg.streamMessage(stream: stream1, topic: 'a', sender: user10, id: 300),
+          eg.streamMessage(stream: stream1, topic: 'a', sender: user10, id: 400),
+        ];
+        setupModel(existingMessages);
+
+        final messages = [
+          eg.streamMessage(stream: stream1, topic: 'a', sender: user10, id: 100),
+          eg.streamMessage(stream: stream1, topic: 'a', sender: user10, id: 200),
+        ];
+        model.handleMessages(messages);
+
+        checkMatchesMessages(model,
+          [...existingMessages, ...messages]..sort((m1, m2) => m1.id.compareTo(m2.id)));


These handleMessages tests are pretty verbose, with a lot of stuff in them that's the same from test to test and isn't what the test is about. I'll push a couple of added commits that factor that out, so that it's a lot easier for the reader to scan these tests and see what they're saying.

gnprice · 2024-07-19T22:32:28Z

test/model/recent_senders_test.dart

+    group('multiple trackers', () {
+      test('batch goes before the existing messages', () {


I don't think it's all that helpful to take the single-tracker tests and make a copy of each of them that distributes across a couple of different topics. What I asked for at #692 (comment) was:

there should also be a couple of test cases that show that handleMessages correctly handles taking messages that belong in several different trackers, and distributing them across those.

These test cases don't really do that. They don't test that handleMessages correctly handles:

having messages in several streams

or messages from several senders

or creating a new tracker where there wasn't one already;

really the one thing they add is testing that it handles getting messages in multiple topics of one stream.

Conversely there isn't a need to repeat the testing of all the different ways the old and new messages in a given tracker can be related; that logic all operates within each tracker separately, so the single-tracker tests cover it.

These tests do add a lot of lines of code. One effect of that is that it takes some work to scan through and see all the things the tests are saying, in order to get the picture of what scenarios are covered and what scenarios aren't. One reason making tests concise and focused (as discussed in comments just above this one) is important is that it helps with detecting missing scenarios, like here.

I'll push another commit replacing these with tests that cover those.

This replaces the "multiple trackers" group with test cases that are written to exercise the various scenarios relevant to the logic in RecentSenders.handleMessages. See also discussion here: zulip#692 (comment)

sm-sayedi · 2024-07-22T20:10:04Z

Thank you @gnprice for the review and the added commits; they're really helpful. New revision pushed. PTAL.

gnprice

Thanks for the revision! No remaining comments on the behavior; just three small comments below, and then this will be ready for merge.

gnprice · 2024-07-23T04:49:59Z

lib/model/recent_senders.dart

+      final sendersByStream = streamSenders[streamId];
+      final messagesBySenderInStream = sendersByStream?[senderId];
+      messagesBySenderInStream?.removeAll(messages);
+      if (messagesBySenderInStream?.maxId == null) sendersByStream?.remove(senderId);
+      if (sendersByStream?.isEmpty ?? false) streamSenders.remove(streamId);


The first and last lines of this stanza can move outside the loop, since they don't depend on senderId or messages.

I also moved the lines related to the topic outside of the loop as they were not dependent on senderId or messages either.

gnprice · 2024-07-23T04:52:21Z

lib/model/recent_senders.dart

+  /// because that's the common case for a message that is received through
+  /// [PerAccountStore.handleEvent]. May take O(n) time in some rare cases.
+  void add(int id) {
+    if (ids.isEmpty || id > maxId!) {


nit:

Suggested change

if (ids.isEmpty || id > maxId!) {

if (ids.isEmpty || id > ids.last) {

The behavior is the same, and since the rest of this function is in terms of ids it makes it a bit easier to reason about how the different pieces of logic within the function relate to each other.

gnprice · 2024-07-23T05:04:14Z

lib/model/recent_senders.dart

+    ids = setUnion(ids, newIds);
+  }
+
+  void removeAll(QueueList<int> idsToRemove) {


nit: this doesn't use that it's a QueueList, so might as well take a general List (and the caller might as well make plain default lists with [], rather than QueueLists)

The motivation was that we use QueueList internally as well as in addAll, so I thought passing a QueueList in removeAll would align with them. Anyway, changed it back to List. 🙂

…necessary In this specific case, this change is not crucial for the test performance as the loop iterations are way less to cause an issue, but it is a good habit to add it as one would copy this part of the code in the future and repurpose it with large loop iterations; in which it would be a test performance issue.

… structures MessageIdTracker data structure is used to keep track of message ids in an ascending sorted list. It is used in RecentSenders data structure. RecentSenders data structure is used to keep track of user messages in topics and streams. Much of this code is transcribed from Zulip web; in particular, from: https://github.com/zulip/zulip/blob/bd04a30bbc6dc5bd7c20940a3d1d34cf8c8c6f28/web/src/recent_senders.ts

The function we're passing this to doesn't notice the order anyway -- it just puts these into sets.

This replaces the "multiple trackers" group with test cases that are written to exercise the various scenarios relevant to the logic in RecentSenders.handleMessages. See also discussion here: zulip#692 (comment)

…ferent Most of these tests don't use the specific stream IDs, user IDs, or message IDs, so cut them out to reduce the number of details for the reader to read. Then also rename the streams, topics, and users so that they're a bit more visually distinct from each other. That helps in seeing what's going on in the tests that are all about which bits of data are the same as, or different from, which other ones.

This will avoid copying the list on each call to `addAll`.

sm-sayedi · 2024-07-23T05:43:53Z

Thanks @gnprice for the review. Revision pushed.

A map of "foos by bar" is a map where each key is a "bar", and each value is a "foo" -- the phrase can be read as an abbreviation of "foos indexed by bar" or "foos, where each foo is indexed by its respective bar". So in this function `messagesByUser` is accurately named because a key represents a user and a value represents some messages. But these other variables should be e.g. `topicsInStream`, not `topicsByStream`. That one isn't a map where the key is a stream and the value is a topic or topics; rather it's a collection of topics (and more data about them) which is the collection specifically of topics in the stream we're acting on.

gnprice · 2024-07-23T17:50:26Z

Looks good — merging! Thanks again for all your work on this. On to #828, the last stage, so that we can get this feature out to users 🙂

I added some small NFC commits on top; please take a look through those:

73c827c recent_senders [nfc]: Rename some foosByBar that mean foosInBar

A map of "foos by bar" is a map where each key is a "bar", and
each value is a "foo" -- the phrase can be read as an abbreviation
of "foos indexed by bar" or "foos, where each foo is indexed by
its respective bar".

So in this function messagesByUser is accurately named because
a key represents a user and a value represents some messages.

But these other variables should be e.g. topicsInStream, not
topicsByStream. That one isn't a map where the key is a stream
and the value is a topic or topics; rather it's a collection of
topics (and more data about them) which is the collection
specifically of topics in the stream we're acting on.

eca8af9 recent_senders [nfc]: Add and revise some docs

dbb2dcf recent_senders [nfc]: Write "sorted ascending"

sm-sayedi added the maintainer review PR ready for review by Zulip maintainers label May 20, 2024

sm-sayedi commented May 20, 2024

View reviewed changes

sm-sayedi force-pushed the issue-228-@-mention-results-with-recent-senders-criterion branch from 1bd73ff to 6dac3d8 Compare May 20, 2024 18:23

chrisbobbe mentioned this pull request May 22, 2024

autocomplete: In user-mention autocomplete results give priority to users in DM conversations #693

Merged

sm-sayedi mentioned this pull request May 22, 2024

autocomplete: Sort user-mention autocomplete results #608

Closed

gnprice added buddy review GSoC buddy review needed. and removed maintainer review PR ready for review by Zulip maintainers labels May 22, 2024

gnprice requested a review from rajveermalviya May 22, 2024 19:23

rajveermalviya reviewed May 23, 2024

View reviewed changes

gnprice added mentor review GSoC mentor review needed. and removed mentor review GSoC mentor review needed. labels May 23, 2024

sm-sayedi force-pushed the issue-228-@-mention-results-with-recent-senders-criterion branch from 6dac3d8 to 0d28448 Compare May 29, 2024 15:35

sm-sayedi requested a review from rajveermalviya May 29, 2024 15:48

sm-sayedi commented May 29, 2024

View reviewed changes

rajveermalviya approved these changes May 30, 2024

View reviewed changes

sm-sayedi added mentor review GSoC mentor review needed. and removed buddy review GSoC buddy review needed. labels May 30, 2024

sm-sayedi requested a review from hackerkid May 30, 2024 14:37

hackerkid approved these changes Jun 1, 2024

View reviewed changes

sm-sayedi removed the mentor review GSoC mentor review needed. label Jun 3, 2024

gnprice added the mentor review GSoC mentor review needed. label Jun 4, 2024

gnprice requested a review from hackerkid June 4, 2024 02:06

sm-sayedi force-pushed the issue-228-@-mention-results-with-recent-senders-criterion branch 3 times, most recently from d6d337c to 2fa9e68 Compare June 11, 2024 19:08

hackerkid reviewed Jun 19, 2024

View reviewed changes

sm-sayedi force-pushed the issue-228-@-mention-results-with-recent-senders-criterion branch from ef93c67 to 3f96e33 Compare July 19, 2024 20:00

sm-sayedi changed the title ~~autocomplete: Add "recent senders criterion" to user-mention autocomplete~~ autocomplete: Introduce data structures for "recent senders criterion" of user-mention autocomplete Jul 19, 2024

sm-sayedi force-pushed the issue-228-@-mention-results-with-recent-senders-criterion branch from 3f96e33 to ddcba66 Compare July 19, 2024 20:15

sm-sayedi commented Jul 19, 2024

View reviewed changes

sm-sayedi mentioned this pull request Jul 19, 2024

autocomplete: Add "recent senders criterion" to user-mention autocomplete #828

Merged

sm-sayedi changed the title ~~autocomplete: Introduce data structures for "recent senders criterion" of user-mention autocomplete~~ model: Introduce data structures for "recent senders criterion" of user-mention autocomplete Jul 19, 2024

gnprice reviewed Jul 19, 2024

View reviewed changes

sm-sayedi force-pushed the issue-228-@-mention-results-with-recent-senders-criterion branch from 0c1edc4 to a208d56 Compare July 22, 2024 20:05

sm-sayedi requested a review from gnprice July 22, 2024 20:10

gnprice reviewed Jul 23, 2024

View reviewed changes

sm-sayedi and others added 8 commits July 23, 2024 10:03

recent_senders test [nfc]: Pull out a checkHandleMessages helper

76d1404

recent_senders test [nfc]: Pull out checkHandleMessagesSingle helper

e875302

recent_senders test [nfc]: Cut redundant sort

3a834c4

The function we're passing this to doesn't notice the order anyway -- it just puts these into sets.

recent_senders test: Test handleMessages implementation

00ebe6e

This replaces the "multiple trackers" group with test cases that are written to exercise the various scenarios relevant to the logic in RecentSenders.handleMessages. See also discussion here: zulip#692 (comment)

recent_senders [nfc]: Make addAll take QueueList instead of List

d3d3774

This will avoid copying the list on each call to `addAll`.

sm-sayedi force-pushed the issue-228-@-mention-results-with-recent-senders-criterion branch from a208d56 to d3d3774 Compare July 23, 2024 05:38

gnprice added 3 commits July 23, 2024 10:31

recent_senders [nfc]: Add and revise some docs

eca8af9

recent_senders [nfc]: Write "sorted ascending"

dbb2dcf

gnprice merged commit dbb2dcf into zulip:main Jul 23, 2024
1 check passed

gnprice mentioned this pull request Aug 22, 2024

autocomplete: Support @-wildcard in user-mention autocomplete #889

Merged

	void handleMessage(Message message) {
	void handleMessageEvent(MessageEvent event) {
	final message = event.message;

	/// Whether stream senders and topic senders are both not empty.
	@visibleForTesting
	bool get debugIsNotEmpty => _streamSenders.isNotEmpty && _topicSenders.isNotEmpty;

		(messagesByUserInStream[(streamId, senderId)] ??= []).add(id);
		(messagesByUserInTopic[(streamId, topic, senderId)] ??= []).add(id);

	(messagesByUserInStream[(streamId, senderId)] ??= []).add(id);
	(messagesByUserInTopic[(streamId, topic, senderId)] ??= []).add(id);
	(messagesByUser[senderId] ??= []).add(id);

		group('multiple trackers', () {
		test('batch goes before the existing messages', () {

	if (ids.isEmpty \|\| id > maxId!) {
	if (ids.isEmpty \|\| id > ids.last) {

model: Introduce data structures for "recent senders criterion" of user-mention autocomplete #692

model: Introduce data structures for "recent senders criterion" of user-mention autocomplete #692

Conversation

sm-sayedi commented May 20, 2024 • edited Loading

Choose a reason for hiding this comment

rajveermalviya May 23, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gnprice commented May 22, 2024

rajveermalviya left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rajveermalviya May 23, 2024 • edited Loading

Choose a reason for hiding this comment

sm-sayedi commented May 29, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rajveermalviya left a comment

Choose a reason for hiding this comment

hackerkid left a comment

Choose a reason for hiding this comment

gnprice commented Jun 4, 2024

sm-sayedi commented Jun 11, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sm-sayedi commented Jul 19, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gnprice left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sm-sayedi commented Jul 22, 2024

gnprice left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sm-sayedi commented Jul 23, 2024

gnprice commented Jul 23, 2024

sm-sayedi commented May 20, 2024 •

edited

Loading

rajveermalviya May 23, 2024 •

edited

Loading

rajveermalviya May 23, 2024 •

edited

Loading