Re-wrap comments with new max line-length

Black won't re-wrap comments because it has no way to determine when
that's a "safe" thing to do (that might break deliberate line-breaks).
So this is a quick pass through to re-wrap most multi-line comments and
docstrings to the new max line-length of 88.

tildes/gunicorn_config.py

@@ -6,9 +6,8 @@ from prometheus_client import multiprocess
 def child_exit(server, worker):  # type: ignore
     """Mark worker processes as dead for Prometheus when the worker exits.
 
-    Note that this uses the child_exit hook instead of worker_exit so that
+    Note that this uses the child_exit hook instead of worker_exit so that it's handled
-    it's handled by the master process (and will still be called if a worker
+    by the master process (and will still be called if a worker crashes).
-    crashes).
     """
     # pylint: disable=unused-argument
     multiprocess.mark_process_dead(worker.pid)

tildes/scripts/breached_passwords.py

@@ -1,17 +1,15 @@
 """Command-line tools for managing a breached-passwords bloom filter.
 
-This tool will help with creating and updating a bloom filter in Redis (using
+This tool will help with creating and updating a bloom filter in Redis (using ReBloom:
-ReBloom: https://github.com/RedisLabsModules/rebloom) to hold hashes for
+https://github.com/RedisLabsModules/rebloom) to hold hashes for passwords that have been
-passwords that have been revealed through data breaches (to prevent users from
+revealed through data breaches (to prevent users from using these passwords here). The
-using these passwords here). The dumps are likely primarily sourced from Troy
+dumps are likely primarily sourced from Troy Hunt's "Pwned Passwords" files:
-Hunt's "Pwned Passwords" files:
 https://haveibeenpwned.com/Passwords
 
-Specifically, the commands in this tool allow building the bloom filter
+Specifically, the commands in this tool allow building the bloom filter somewhere else,
-somewhere else, then the RDB file can be transferred to the production server.
+then the RDB file can be transferred to the production server.  Note that it is expected
-Note that it is expected that a separate redis server instance is running
+that a separate redis server instance is running solely for holding this bloom filter.
-solely for holding this bloom filter. Replacing the RDB file will result in all
+Replacing the RDB file will result in all other keys being lost.
-other keys being lost.
 
 Expected usage of this tool should look something like:
 
@@ -20,8 +18,8 @@ On the machine building the bloom filter:
     python breached_passwords.py addhashes pwned-passwords-1.0.txt
     python breached_passwords.py addhashes pwned-passwords-update-1.txt
 
-Then the RDB file can simply be transferred to the production server,
+Then the RDB file can simply be transferred to the production server, overwriting any
-overwriting any previous RDB file.
+previous RDB file.
 
 """
 
@@ -90,14 +88,13 @@ def validate_init_error_rate(ctx: Any, param: Any, value: Any) -> float:
 def init(estimate: int, error_rate: float) -> None:
     """Initialize a new bloom filter (destroying any existing one).
 
-    It generally shouldn't be necessary to re-init a new bloom filter very
+    It generally shouldn't be necessary to re-init a new bloom filter very often with
-    often with this command, only if the previous one was created with too low
+    this command, only if the previous one was created with too low of an estimate for
-    of an estimate for number of passwords, or to change to a different false
+    number of passwords, or to change to a different false positive rate. For choosing
-    positive rate. For choosing an estimate value, according to the ReBloom
+    an estimate value, according to the ReBloom documentation: "Performance will begin
-    documentation: "Performance will begin to degrade after adding more items
+    to degrade after adding more items than this number. The actual degradation will
-    than this number. The actual degradation will depend on how far the limit
+    depend on how far the limit has been exceeded. Performance will degrade linearly as
-    has been exceeded. Performance will degrade linearly as the number of
+    the number of entries grow exponentially."
-    entries grow exponentially."
     """
     REDIS.delete(BREACHED_PASSWORDS_BF_KEY)
 
@@ -115,8 +112,8 @@ def init(estimate: int, error_rate: float) -> None:
 def addhashes(filename: str) -> None:
     """Add all hashes from a file to the bloom filter.
 
-    This uses the method of generating commands in Redis protocol and feeding
+    This uses the method of generating commands in Redis protocol and feeding them into
-    them into an instance of `redis-cli --pipe`, as recommended in
+    an instance of `redis-cli --pipe`, as recommended in
     https://redis.io/topics/mass-insert
     """
     # make sure the key exists and is a bloom filter
@@ -146,9 +143,9 @@ def addhashes(filename: str) -> None:
     for count, line in enumerate(open(filename), start=1):
         hashval = line.strip().lower()
 
-        # the Pwned Passwords hash lists now have a frequency count for each
+        # the Pwned Passwords hash lists now have a frequency count for each hash, which
-        # hash, which is separated from the hash with a colon, so we need to
+        # is separated from the hash with a colon, so we need to handle that if it's
-        # handle that if it's present
+        # present
         hashval = hashval.split(":")[0]
 
         command = generate_redis_protocol("BF.ADD", BREACHED_PASSWORDS_BF_KEY, hashval)

tildes/scripts/clean_private_data.py

@@ -24,8 +24,8 @@ RETENTION_PERIOD = timedelta(days=30)
 def clean_all_data(config_path: str) -> None:
     """Clean all private/deleted data.
 
-    This should generally be the only function called in most cases, and will
+    This should generally be the only function called in most cases, and will initiate
-    initiate the full cleanup process.
+    the full cleanup process.
     """
     db_session = get_session_from_config(config_path)
 
@@ -54,8 +54,8 @@ class DataCleaner:
     def delete_old_log_entries(self) -> None:
         """Delete all log entries older than the retention cutoff.
 
-        Note that this will also delete all entries from the child tables that
+        Note that this will also delete all entries from the child tables that inherit
-        inherit from Log (LogTopics, etc.).
+        from Log (LogTopics, etc.).
         """
         deleted = (
             self.db_session.query(Log)
@@ -78,8 +78,8 @@ class DataCleaner:
     def clean_old_deleted_comments(self) -> None:
         """Clean the data of old deleted comments.
 
-        Change the comment's author to the "unknown user" (id 0), and delete
+        Change the comment's author to the "unknown user" (id 0), and delete its
-        its contents.
+        contents.
         """
         updated = (
             self.db_session.query(Comment)
@@ -98,8 +98,8 @@ class DataCleaner:
     def clean_old_deleted_topics(self) -> None:
         """Clean the data of old deleted topics.
 
-        Change the topic's author to the "unknown user" (id 0), and delete its
+        Change the topic's author to the "unknown user" (id 0), and delete its title,
-        title, contents, tags, and metadata.
+        contents, tags, and metadata.
         """
         updated = (
             self.db_session.query(Topic)

tildes/scripts/initialize_db.py

@@ -24,8 +24,8 @@ def initialize_db(config_path: str, alembic_config_path: Optional[str] = None) -
 
     run_sql_scripts_in_dir("sql/init/", engine)
 
-    # if an Alembic config file wasn't specified, assume it's alembic.ini in
+    # if an Alembic config file wasn't specified, assume it's alembic.ini in the same
-    # the same directory
+    # directory
     if not alembic_config_path:
         path = os.path.split(config_path)[0]
         alembic_config_path = os.path.join(path, "alembic.ini")

tildes/tests/conftest.py

@@ -66,9 +66,9 @@ def overall_db_session(pyramid_config):
 
     create_tables(session.connection())
 
-    # SQL init scripts need to be executed "manually" instead of using psql
+    # SQL init scripts need to be executed "manually" instead of using psql like the
-    # like the normal database init process does, since the tables only exist
+    # normal database init process does, since the tables only exist inside this
-    # inside this transaction
+    # transaction
     init_scripts_dir = "sql/init/"
     for root, _, files in os.walk(init_scripts_dir):
         sql_files = [filename for filename in files if filename.endswith(".sql")]
@@ -76,8 +76,8 @@ def overall_db_session(pyramid_config):
             with open(os.path.join(root, sql_file)) as current_file:
                 session.execute(current_file.read())
 
-    # convert the Session to the wrapper class to enforce staying inside
+    # convert the Session to the wrapper class to enforce staying inside nested
-    # nested transactions in the test functions
+    # transactions in the test functions
     session.__class__ = NestedSessionWrapper
 
     yield session
@@ -110,8 +110,8 @@ def db(overall_db_session):
 @fixture(scope="session", autouse=True)
 def overall_redis_session():
     """Create a session-level connection to a temporary redis server."""
-    # list of redis modules that need to be loaded (would be much nicer to do
+    # list of redis modules that need to be loaded (would be much nicer to do this
-    # this automatically somehow, maybe reading from the real redis.conf?)
+    # automatically somehow, maybe reading from the real redis.conf?)
     redis_modules = ["/opt/redis-cell/libredis_cell.so"]
 
     with RedisServer() as temp_redis_server:
@@ -134,9 +134,9 @@ def redis(overall_redis_session):
 @fixture(scope="session", autouse=True)
 def session_user(sdb):
     """Create a user named 'SessionUser' in the db for test session."""
-    # note that some tests may depend on this username/password having these
+    # note that some tests may depend on this username/password having these specific
-    # specific values, so make sure to search for and update those tests if you
+    # values, so make sure to search for and update those tests if you change the
-    # change the username or password for any reason
+    # username or password for any reason
     user = User("SessionUser", "session user password")
     sdb.add(user)
     sdb.commit()
@@ -148,8 +148,8 @@ def session_user(sdb):
 def session_user2(sdb):
     """Create a second user named 'OtherUser' in the db for test session.
 
-    This is useful for cases where two different users are needed, such as
+    This is useful for cases where two different users are needed, such as when testing
-    when testing private messages.
+    private messages.
     """
     user = User("OtherUser", "other user password")
     sdb.add(user)
@@ -173,8 +173,8 @@ def base_app(overall_redis_session, sdb):
     """Configure a base WSGI app that webtest can create TestApps based on."""
     testing_app = get_app("development.ini")
 
-    # replace the redis connection used by the redis-sessions library with a
+    # replace the redis connection used by the redis-sessions library with a connection
-    # connection to the temporary server for this test session
+    # to the temporary server for this test session
     testing_app.app.registry._redis_sessions = overall_redis_session
 
     def redis_factory(request):
@@ -183,8 +183,8 @@ def base_app(overall_redis_session, sdb):
 
     testing_app.app.registry["redis_connection_factory"] = redis_factory
 
-    # replace the session factory function with one that will return the
+    # replace the session factory function with one that will return the testing db
-    # testing db session (inside a nested transaction)
+    # session (inside a nested transaction)
     def session_factory():
         return sdb
 
@@ -196,8 +196,8 @@ def base_app(overall_redis_session, sdb):
 @fixture(scope="session")
 def webtest(base_app):
     """Create a webtest TestApp and log in as the SessionUser account in it."""
-    # create the TestApp - note that specifying wsgi.url_scheme is necessary
+    # create the TestApp - note that specifying wsgi.url_scheme is necessary so that the
-    # so that the secure cookies from the session library will work
+    # secure cookies from the session library will work
     app = TestApp(
         base_app, extra_environ={"wsgi.url_scheme": "https"}, cookiejar=CookieJar()
     )

tildes/tests/test_comment_user_mentions.py

@@ -68,8 +68,8 @@ def test_prevent_duplicate_notifications(db, user_list, topic):
     """Test that notifications are cleaned up for edits.
 
     Flow:
-        1. A comment is created by user A that mentions user B. Notifications
+        1. A comment is created by user A that mentions user B. Notifications are
-        are generated, and yield A mentioning B.
+           generated, and yield A mentioning B.
         2. The comment is edited to mention C and not B.
         3. The comment is edited to mention B and C.
         4. The comment is deleted.

tildes/tests/test_ratelimit.py

@@ -104,8 +104,8 @@ def test_time_until_retry(redis):
     period = timedelta(seconds=60)
     limit = 2
 
-    # create an action with no burst allowed, which will force the actions to
+    # create an action with no burst allowed, which will force the actions to be spaced
-    # be spaced "evenly" across the limit
+    # "evenly" across the limit
     action = RateLimitedAction(
         "test", period=period, limit=limit, max_burst=1, redis=redis
     )

tildes/tests/test_simplestring_field.py

@@ -13,8 +13,8 @@ class SimpleStringTestSchema(Schema):
 def process_string(string):
     """Deserialize a string with the field and return the "final" version.
 
-    This also works for testing validation since .load() will raise a
+    This also works for testing validation since .load() will raise a ValidationError if
-    ValidationError if an invalid string is attempted.
+    an invalid string is attempted.
     """
     schema = SimpleStringTestSchema(strict=True)
     result = schema.load({"subject": string})

tildes/tests/test_string.py

@@ -96,17 +96,17 @@ def test_multibyte_url_slug():
 
 def test_multibyte_conservative_truncation():
     """Ensure truncating a multibyte url slug won't massively shorten it."""
-    # this string has a comma as the 6th char which will be converted to an
+    # this string has a comma as the 6th char which will be converted to an underscore,
-    # underscore, so if truncation amount isn't restricted, it would result in
+    # so if truncation amount isn't restricted, it would result in a 46-char slug
-    # a 46-char slug instead of the full 100.
+    # instead of the full 100.
     original = "パイソンは、汎用のプログラミング言語である"
     assert len(convert_to_url_slug(original, 100)) == 100
 
 
 def test_multibyte_whole_character_truncation():
     """Ensure truncation happens at the edge of a multibyte character."""
-    # each of these characters url-encodes to 3 bytes = 9 characters each, so
+    # each of these characters url-encodes to 3 bytes = 9 characters each, so only the
-    # only the first character should be included for all lengths from 9 - 17
+    # first character should be included for all lengths from 9 - 17
     original = "コード"
     for limit in range(9, 18):
         assert convert_to_url_slug(original, limit) == "%E3%82%B3"

tildes/tests/webtests/test_user_page.py

@@ -1,9 +1,9 @@
 def test_loggedout_username_leak(webtest_loggedout, session_user):
     """Ensure responses from existing and nonexistent users are the same.
 
-    Since logged-out users are currently blocked from seeing user pages, this
+    Since logged-out users are currently blocked from seeing user pages, this makes sure
-    makes sure that there isn't a data leak where it's possible to tell if a
+    that there isn't a data leak where it's possible to tell if a particular username
-    particular username exists or not.
+    exists or not.
     """
     existing_user = webtest_loggedout.get(
         "/user/" + session_user.username, expect_errors=True

tildes/tildes/__init__.py

@@ -38,8 +38,8 @@ def main(global_config: Dict[str, str], **settings: str) -> PrefixMiddleware:
 
     config.add_request_method(is_safe_request_method, "is_safe_method", reify=True)
 
-    # Add the request.redis request method to access a redis connection. This
+    # Add the request.redis request method to access a redis connection. This is done in
-    # is done in a bit of a strange way to support being overridden in tests.
+    # a bit of a strange way to support being overridden in tests.
     config.registry["redis_connection_factory"] = get_redis_connection
     # pylint: disable=unnecessary-lambda
     config.add_request_method(
@@ -125,8 +125,8 @@ def current_listing_base_url(
 ) -> str:
     """Return the "base" url for the current listing route.
 
-    The "base" url represents the current listing, including any filtering
+    The "base" url represents the current listing, including any filtering options (or
-    options (or the fact that filters are disabled).
+    the fact that filters are disabled).
 
     The `query` argument allows adding query variables to the generated url.
     """

tildes/tildes/api.py

@@ -19,8 +19,8 @@ class APIv0(Service):
 
         super().__init__(name=name, path=path, **kwargs)
 
-        # Service.__init__ does this setup to support config.scan(), but it
+        # Service.__init__ does this setup to support config.scan(), but it doesn't seem
-        # doesn't seem to inherit properly, so it needs to be done again here
+        # to inherit properly, so it needs to be done again here
         def callback(context: Any, name: Any, obj: Any) -> None:
             # pylint: disable=unused-argument
             config = context.config.with_package(info.module)  # noqa

tildes/tildes/auth.py

@@ -15,10 +15,10 @@ from tildes.models.user import User
 class DefaultRootFactory:
     """Default root factory to grant everyone 'view' permission by default.
 
-    Note that this will only be applied in cases where a view does not have a
+    Note that this will only be applied in cases where a view does not have a factory
-    factory specified at all (so request.context doesn't have a meaningful
+    specified at all (so request.context doesn't have a meaningful value). Any classes
-    value). Any classes that could be returned by a root factory must have
+    that could be returned by a root factory must have an __acl__ defined, they will not
-    an __acl__ defined, they will not "fall back" to this one.
+    "fall back" to this one.
     """
 
     __acl__ = ((Allow, Everyone, "view"),)
@@ -42,8 +42,8 @@ def get_authenticated_user(request: Request) -> Optional[User]:
 def auth_callback(user_id: int, request: Request) -> Optional[Sequence[str]]:
     """Return authorization principals for a user_id from the session.
 
-    This is a callback function needed by SessionAuthenticationPolicy. It
+    This is a callback function needed by SessionAuthenticationPolicy. It should return
-    should return None if the user_id does not exist (such as a deleted user).
+    None if the user_id does not exist (such as a deleted user).
     """
     if not request.user:
         return None
@@ -69,14 +69,14 @@ def includeme(config: Configurator) -> None:
     # make all views require "view" permission unless specifically overridden
     config.set_default_permission("view")
 
-    # replace the default root factory with a custom one to more easily support
+    # replace the default root factory with a custom one to more easily support the
-    # the default permission
+    # default permission
     config.set_root_factory(DefaultRootFactory)
 
-    # Set the authorization policy to a custom one that always returns a
+    # Set the authorization policy to a custom one that always returns a "denied" result
-    # "denied" result if the user isn't logged in. When overall site access is
+    # if the user isn't logged in. When overall site access is no longer being
-    # no longer being restricted, the AuthorizedOnlyPolicy class can just be
+    # restricted, the AuthorizedOnlyPolicy class can just be replaced with the standard
-    # replaced with the standard ACLAuthorizationPolicy
+    # ACLAuthorizationPolicy
     config.set_authorization_policy(AuthorizedOnlyPolicy())
 
     config.set_authentication_policy(

tildes/tildes/database.py

@@ -56,20 +56,18 @@ def includeme(config: Configurator) -> None:
 
     Currently adds:
 
-    * request.db_session - the db session for the current request, managed by
+    * request.db_session - db session for the current request, managed by pyramid_tm.
-      pyramid_tm.
+    * request.query() - a factory method that will return a ModelQuery or subclass for
-    * request.query() - a factory method that will return a ModelQuery or
+      querying the model class supplied. This will generally be used generatively,
-      subclass for querying the model class supplied. This will generally be
+      similar to standard SQLALchemy session.query(...).
-      used generatively, similar to standard SQLALchemy session.query(...).
+    * request.obtain_lock() - obtains a transaction-level advisory lock from PostgreSQL.
-    * request.obtain_lock() - obtains a transaction-level advisory lock from
-      PostgreSQL.
     """
     settings = config.get_settings()
 
-    # Enable pyramid_tm's default_commit_veto behavior, which will abort the
+    # Enable pyramid_tm's default_commit_veto behavior, which will abort the transaction
-    # transaction if the response code starts with 4 or 5. The main benefit of
+    # if the response code starts with 4 or 5. The main benefit of this is to avoid
-    # this is to avoid aborting on exceptions that don't actually indicate a
+    # aborting on exceptions that don't actually indicate a problem, such as a HTTPFound
-    # problem, such as a HTTPFound 302 redirect.
+    # 302 redirect.
     settings["tm.commit_veto"] = "pyramid_tm.default_commit_veto"
 
     config.include("pyramid_tm")

tildes/tildes/enums.py

@@ -68,9 +68,9 @@ class TopicSortOption(enum.Enum):
     def descending_description(self) -> str:
         """Describe this sort option when used in a "descending" order.
 
-        For example, the "votes" sort has a description of "most votes", since
+        For example, the "votes" sort has a description of "most votes", since using
-        using that sort in descending order means that topics with the most
+        that sort in descending order means that topics with the most votes will be
-        votes will be listed first.
+        listed first.
         """
         if self.name == "NEW":
             return "newest"

tildes/tildes/json.py

@@ -13,8 +13,8 @@ from tildes.models.user import User
 def serialize_model(model_item: DatabaseModel, request: Request) -> dict:
     """Return serializable data for a DatabaseModel item.
 
-    Uses the .schema class attribute to serialize a model by using its
+    Uses the .schema class attribute to serialize a model by using its corresponding
-    corresponding marshmallow schema.
+    marshmallow schema.
     """
     # pylint: disable=unused-argument
     return model_item.schema.dump(model_item)

tildes/tildes/lib/__init__.py

@@ -1,9 +1,8 @@
 """Contains the overall "library" for the application.
 
-Defining constants, behavior, etc. inside modules here (as opposed to other
+Defining constants, behavior, etc. inside modules here (as opposed to other locations
-locations such as in models) is encouraged, since it often makes it simpler to
+such as in models) is encouraged, since it often makes it simpler to import elsewhere
-import elsewhere for tests, when only a specific constant value is needed, etc.
+for tests, when only a specific constant value is needed, etc.
 
-Modules here should *never* import anything from models, to avoid circular
+Modules here should *never* import anything from models, to avoid circular dependencies.
-dependencies.
 """

tildes/tildes/lib/amqp.py

@@ -13,15 +13,14 @@ from tildes.lib.database import get_session_from_config
 class PgsqlQueueConsumer(AbstractConsumer):
     """Base class for consumers of events sent from PostgreSQL via rabbitmq.
 
-    This class is intended to be used in a completely "stand-alone" manner,
+    This class is intended to be used in a completely "stand-alone" manner, such as
-    such as inside a script being run separately as a background job. As such,
+    inside a script being run separately as a background job. As such, it also includes
-    it also includes connecting to rabbitmq, declaring the underlying queue and
+    connecting to rabbitmq, declaring the underlying queue and bindings, and
-    bindings, and (optionally) connecting to the database to be able to fetch
+    (optionally) connecting to the database to be able to fetch and modify data as
-    and modify data as necessary. It relies on the environment variable
+    necessary. It relies on the environment variable INI_FILE being set.
-    INI_FILE being set.
+    Note that all messages received by these consumers are expected to be in JSON
-
+    format.
-    Note that all messages received by these consumers are expected to be in
-    JSON format.
     """
 
     PGSQL_EXCHANGE_NAME = "pgsql_events"

tildes/tildes/lib/database.py

@@ -35,9 +35,9 @@ def obtain_transaction_lock(
 ) -> None:
     """Obtain a transaction-level advisory lock from PostgreSQL.
 
-    The lock_space arg must be either None or the name of one of the members of
+    The lock_space arg must be either None or the name of one of the members of the
-    the LockSpaces enum (case-insensitive). Contention for a lock will only
+    LockSpaces enum (case-insensitive). Contention for a lock will only occur when both
-    occur when both lock_space and lock_value have the same values.
+    lock_space and lock_value have the same values.
     """
     if lock_space:
         try:
@@ -125,8 +125,8 @@ class ArrayOfLtree(ARRAY):  # pylint: disable=too-many-ancestors
     class comparator_factory(ARRAY.comparator_factory):
         """Add custom comparison functions.
 
-        The ancestor_of and descendant_of functions are supported by LtreeType,
+        The ancestor_of and descendant_of functions are supported by LtreeType, so this
-        so this duplicates them here so they can be used on ArrayOfLtree too.
+        duplicates them here so they can be used on ArrayOfLtree too.
         """
 
         def ancestor_of(self, other):  # type: ignore

tildes/tildes/lib/datetime.py

@@ -43,9 +43,8 @@ class SimpleHoursPeriod:
     def __str__(self) -> str:
         """Return a representation of the period as a string.
 
-        Will be of the form "4 hours", "2 days", "1 day, 6 hours", etc. except
+        Will be of the form "4 hours", "2 days", "1 day, 6 hours", etc. except for the
-        for the special case of exactly "1 day", which is replaced with "24
+        special case of exactly "1 day", which is replaced with "24 hours".
-        hours".
         """
         string = human(self.timedelta, past_tense="{}")
         if string == "1 day":
@@ -63,8 +62,8 @@ class SimpleHoursPeriod:
     def as_short_form(self) -> str:
         """Return a representation of the period as a "short form" string.
 
-        Uses "hours" representation unless the period is an exact multiple of
+        Uses "hours" representation unless the period is an exact multiple of 24 hours
-        24 hours (except for 24 hours itself).
+        (except for 24 hours itself).
         """
         if self.hours % 24 == 0 and self.hours != 24:
             return "{}d".format(self.hours // 24)
@@ -80,14 +79,13 @@ def utc_now() -> datetime:
 def descriptive_timedelta(target: datetime, abbreviate: bool = False) -> str:
     """Return a descriptive string for how long ago a datetime was.
 
-    The returned string will be of a format like "4 hours ago" or
+    The returned string will be of a format like "4 hours ago" or "3 hours, 21 minutes
-    "3 hours, 21 minutes ago". The second "precision level" is only added if
+    ago". The second "precision level" is only added if it will be at least minutes, and
-    it will be at least minutes, and only one "level" below the first unit.
+    only one "level" below the first unit.  That is, you'd never see anything like "4
-    That is, you'd never see anything like "4 hours, 5 seconds ago" or
+    hours, 5 seconds ago" or "2 years, 3 hours ago".
-    "2 years, 3 hours ago".
 
-    If `abbreviate` is true, the units will be shortened to return a string
+    If `abbreviate` is true, the units will be shortened to return a string like
-    like "12h 28m ago" instead of "12 hours, 28 minutes ago".
+    "12h 28m ago" instead of "12 hours, 28 minutes ago".
 
     A time of less than a second returns "a moment ago".
     """

tildes/tildes/lib/hash.py

@@ -3,10 +3,10 @@
 from argon2 import PasswordHasher
 from argon2.exceptions import VerifyMismatchError
 
-# These parameter values were chosen to achieve a hash-verification time of
+# These parameter values were chosen to achieve a hash-verification time of about 10ms
-# about 10ms on the current production server. They can be updated to different
+# on the current production server. They can be updated to different values if the
-# values if the server changes (consider upgrading old password hashes on login
+# server changes (consider upgrading old password hashes on login as well if that
-# as well if that happens).
+# happens).
 ARGON2_TIME_COST = 4
 ARGON2_MEMORY_COST = 8092
 

tildes/tildes/lib/id.py

@@ -17,11 +17,10 @@ def id_to_id36(id_val: int) -> str:
     # the "alphabet" of our ID36s - 0-9 followed by a-z
     alphabet = string.digits + string.ascii_lowercase
 
-    # Repeatedly use divmod() on the value, which returns the quotient and
+    # Repeatedly use divmod() on the value, which returns the quotient and remainder of
-    # remainder of each integer division - divmod(a, b) == (a // b, a % b).
+    # each integer division - divmod(a, b) == (a // b, a % b). The remainder of each
-    # The remainder of each division works as an index into the alphabet, and
+    # division works as an index into the alphabet, and doing this repeatedly will build
-    # doing this repeatedly will build up our ID36 string in reverse order
+    # up our ID36 string in reverse order (with the least-significant "digit" first).
-    # (with the least-significant "digit" first).
     quotient, index = divmod(id_val, 36)
     while quotient != 0:
         reversed_chars.append(alphabet[index])

tildes/tildes/lib/markdown.py

@@ -79,18 +79,18 @@ HTML_ATTRIBUTE_WHITELIST = {
 }
 PROTOCOL_WHITELIST = ("http", "https")
 
-# Regex that finds ordered list markdown that was probably accidental - ones
+# Regex that finds ordered list markdown that was probably accidental - ones being
-# being initiated by anything except "1."
+# initiated by anything except "1."
 BAD_ORDERED_LIST_REGEX = re.compile(
     r"((?:\A|\n\n)"  # Either the start of the entire text, or a new paragraph
     r"(?!1\.)\d+)"  # A number that isn't "1"
     r"\.\s"  # Followed by a period and a space
 )
 
-# Type alias for the "namespaced attr dict" used inside bleach.linkify
+# Type alias for the "namespaced attr dict" used inside bleach.linkify callbacks. This
-# callbacks. This looks pretty ridiculous, but it's a dict where the keys are
+# looks pretty ridiculous, but it's a dict where the keys are namespaced attr names,
-# namespaced attr names, like `(None, 'href')`, and there's also a `_text`
+# like `(None, 'href')`, and there's also a `_text` key for getting the innerText of the
-# key for getting the innerText of the <a> tag.
+# <a> tag.
 NamespacedAttrDict = Dict[Union[Tuple[Optional[str], str], str], str]  # noqa
 
 
@@ -155,15 +155,14 @@ def preprocess_markdown(markdown: str) -> str:
 def escape_accidental_ordered_lists(markdown: str) -> str:
     """Escape markdown that's probably an accidental ordered list.
 
-    It's a common markdown mistake to accidentally start a numbered list, by
+    It's a common markdown mistake to accidentally start a numbered list, by beginning a
-    beginning a post or paragraph with a number followed by a period. For
+    post or paragraph with a number followed by a period. For example, someone might try
-    example, someone might try to write "1975. It was a long time ago.", and
+    to write "1975. It was a long time ago.", and the result will be a comment that says
-    the result will be a comment that says "1. It was a long time ago." since
+    "1. It was a long time ago." since that gets parsed into a numbered list.
-    that gets parsed into a numbered list.
 
-    This fixes that quirk of markdown by escaping anything that would start a
+    This fixes that quirk of markdown by escaping anything that would start a numbered
-    numbered list except for "1. ". This will cause a few other edge cases, but
+    list except for "1. ". This will cause a few other edge cases, but I believe they're
-    I believe they're less common/important than fixing this common error.
+    less common/important than fixing this common error.
     """
     return BAD_ORDERED_LIST_REGEX.sub(r"\1\\. ", markdown)
 
@@ -205,24 +204,24 @@ def apply_linkification(html: str, skip_tags: Optional[List[str]] = None) -> str
 class LinkifyFilter(Filter):
     """html5lib Filter to convert custom text patterns to links.
 
-    This replaces references to group paths and usernames with links to the
+    This replaces references to group paths and usernames with links to the relevant
-    relevant pages.
+    pages.
 
-    This implementation is based heavily on the linkify implementation from
+    This implementation is based heavily on the linkify implementation from the Bleach
-    the Bleach library.
+    library.
     """
 
-    # Regex that finds probable references to groups. This isn't "perfect",
+    # Regex that finds probable references to groups. This isn't "perfect", just a first
-    # just a first pass to find likely candidates. The validity of the group
+    # pass to find likely candidates. The validity of the group path is checked more
-    # path is checked more carefully later.
+    # carefully later.
-    # Note: currently specifically excludes paths immediately followed by a
+    # Note: currently specifically excludes paths immediately followed by a tilde, but
-    # tilde, but this may be possible to remove once strikethrough is
+    # this may be possible to remove once strikethrough is implemented (since that's
-    # implemented (since that's probably what they were trying to do)
+    # probably what they were trying to do)
     GROUP_REFERENCE_REGEX = re.compile(r"(?<!\w)~([\w.]+)\b(?!~)")
 
-    # Regex that finds probable references to users. As above, this isn't
+    # Regex that finds probable references to users. As above, this isn't "perfect"
-    # "perfect" either but works as an initial pass with the validity of
+    # either but works as an initial pass with the validity of the username checked more
-    # the username checked more carefully later.
+    # carefully later.
     USERNAME_REFERENCE_REGEX = re.compile(r"(?<!\w)(?:/?u/|@)([\w-]+)\b")
 
     def __init__(
@@ -230,8 +229,8 @@ class LinkifyFilter(Filter):
     ) -> None:
         """Initialize a linkification filter to apply to HTML.
 
-        The skip_tags argument can be a list of tag names, and the contents of
+        The skip_tags argument can be a list of tag names, and the contents of any of
-        any of those tags will be excluded from linkification.
+        those tags will be excluded from linkification.
         """
         super().__init__(source)
         self.skip_tags = skip_tags or []
@@ -248,28 +247,27 @@ class LinkifyFilter(Filter):
                 token["type"] in ("StartTag", "EmptyTag")
                 and token["name"] in self.skip_tags
             ):
-                # if this is the start of a tag we want to skip, add it to the
+                # if this is the start of a tag we want to skip, add it to the list of
-                # list of skipped tags that we're currently inside
+                # skipped tags that we're currently inside
                 inside_skipped_tags.append(token["name"])
             elif inside_skipped_tags:
-                # if we're currently inside any skipped tags, the only thing we
+                # if we're currently inside any skipped tags, the only thing we want to
-                # want to do is look for all the end tags we need to be able to
+                # do is look for all the end tags we need to be able to finish skipping
-                # finish skipping
                 if token["type"] == "EndTag":
                     try:
                         inside_skipped_tags.remove(token["name"])
                     except ValueError:
                         pass
             elif token["type"] == "Characters":
-                # this is only reachable if inside_skipped_tags is empty, so
+                # this is only reachable if inside_skipped_tags is empty, so this is a
-                # this is a text token not inside a skipped tag - do the actual
+                # text token not inside a skipped tag - do the actual linkification
-                # linkification replacements
+                # replacements
-
+                # Note: doing the two replacements "iteratively" like this only works
-                # Note: doing the two replacements "iteratively" like this only
+                # because they are "disjoint" and we know they're not competing to
-                # works because they are "disjoint" and we know they're not
+                # replace the same text. If more replacements are added in the future
-                # competing to replace the same text. If more replacements are
+                # that might conflict with each other, this will need to be reworked
-                # added in the future that might conflict with each other, this
+                # somehow.
-                # will need to be reworked somehow.
                 replaced_tokens = self._linkify_tokens(
                     [token],
                     filter_regex=self.GROUP_REFERENCE_REGEX,
@@ -281,13 +279,13 @@ class LinkifyFilter(Filter):
                     linkify_function=self._tokenize_username_match,
                 )
 
-                # yield all the tokens returned from the replacement process
+                # yield all the tokens returned from the replacement process (will be
-                # (will be just the original token if nothing was replaced)
+                # just the original token if nothing was replaced)
                 for new_token in replaced_tokens:
                     yield new_token
 
-                # we either yielded new tokens or the original one already, so
+                # we either yielded new tokens or the original one already, so we don't
-                # we don't want to fall through and yield the original again
+                # want to fall through and yield the original again
                 continue
 
             yield token
@@ -298,11 +296,11 @@ class LinkifyFilter(Filter):
     ) -> List[dict]:
         """Check tokens for text that matches a regex and linkify it.
 
-        The `filter_regex` argument should be a compiled pattern that will be
+        The `filter_regex` argument should be a compiled pattern that will be applied to
-        applied to the text in all of the supplied tokens. If any matches are
+        the text in all of the supplied tokens. If any matches are found, they will each
-        found, they will each be used to call `linkify_function`, which will
+        be used to call `linkify_function`, which will validate the match and convert it
-        validate the match and convert it back into tokens (representing an <a>
+        back into tokens (representing an <a> tag if it is valid for linkifying, or just
-        tag if it is valid for linkifying, or just text if not).
+        text if not).
         """
         new_tokens = []
 
@@ -316,8 +314,8 @@ class LinkifyFilter(Filter):
             current_index = 0
 
             for match in filter_regex.finditer(original_text):
-                # if there were some characters between the previous match and
+                # if there were some characters between the previous match and this one,
-                # this one, add a token containing those first
+                # add a token containing those first
                 if match.start() > current_index:
                     new_tokens.append(
                         {
@@ -333,8 +331,8 @@ class LinkifyFilter(Filter):
                 # move the progress marker up to the end of this match
                 current_index = match.end()
 
-            # if there's still some text left over, add one more token for it
+            # if there's still some text left over, add one more token for it (this will
-            # (this will be the entire thing if there weren't any matches)
+            # be the entire thing if there weren't any matches)
             if current_index < len(original_text):
                 new_tokens.append(
                     {"type": "Characters", "data": original_text[current_index:]}
@@ -345,14 +343,14 @@ class LinkifyFilter(Filter):
     @staticmethod
     def _tokenize_group_match(match: Match) -> List[dict]:
         """Convert a potential group reference into HTML tokens."""
-        # convert the potential group path to lowercase to allow people to use
+        # convert the potential group path to lowercase to allow people to use incorrect
-        # incorrect casing but still have it link properly
+        # casing but still have it link properly
         group_path = match[1].lower()
 
-        # Even though they're technically valid paths, we don't want to linkify
+        # Even though they're technically valid paths, we don't want to linkify things
-        # things like "~10" or "~4.5" since that's just going to be someone
+        # like "~10" or "~4.5" since that's just going to be someone using it in the
-        # using it in the "approximately" sense. So if the path consists of
+        # "approximately" sense. So if the path consists of only numbers and/or periods,
-        # only numbers and/or periods, we won't linkify it
+        # we won't linkify it
         is_numeric = all(char in "0123456789." for char in group_path)
 
         # if it's a valid group path and not totally numeric, convert to <a>

tildes/tildes/lib/password.py

@@ -22,6 +22,6 @@ def is_breached_password(password: str) -> bool:
             redis.execute_command("BF.EXISTS", BREACHED_PASSWORDS_BF_KEY, hashed)
         )
     except (ConnectionError, ResponseError):
-        # server isn't running, bloom filter doesn't exist or the key is a
+        # server isn't running, bloom filter doesn't exist or the key is a different
-        # different data type
+        # data type
         return False

tildes/tildes/lib/ratelimit.py

@@ -19,9 +19,8 @@ class RateLimitError(Exception):
 class RateLimitResult:
     """The result from a rate-limit check.
 
-    Includes data relating to whether the action should be allowed or blocked,
+    Includes data relating to whether the action should be allowed or blocked, how much
-    how much of the limit is remaining, how long until the action can be
+    of the limit is remaining, how long until the action can be retried, etc.
-    retried, etc.
     """
 
     def __init__(
@@ -100,23 +99,22 @@ class RateLimitResult:
     def merged_result(cls, results: Sequence["RateLimitResult"]) -> "RateLimitResult":
         """Merge any number of RateLimitResults into a single result.
 
-        Basically, the merged result should be the "most restrictive"
+        Basically, the merged result should be the "most restrictive" combination of all
-        combination of all the source results. That is, it should only allow
+        the source results. That is, it should only allow the action if *all* of the
-        the action if *all* of the source results would allow it, the limit
+        source results would allow it, the limit counts should be the lowest of the set,
-        counts should be the lowest of the set, and the waiting times should
+        and the waiting times should be the highest of the set.
-        be the highest of the set.
 
-        Note: I think the behavior for time_until_max is not truly correct, but
+        Note: I think the behavior for time_until_max is not truly correct, but it
-        it should be reasonable for now. Consider a situation like two
+        should be reasonable for now. Consider a situation like two "overlapping" limits
-        "overlapping" limits of 10/min and 100/hour and what the time_until_max
+        of 10/min and 100/hour and what the time_until_max value of the combination
-        value of the combination should be. It might be a bit tricky.
+        should be. It might be a bit tricky.
         """
         # if there's only one result, just return that one
         if len(results) == 1:
             return results[0]
 
-        # time_until_retry is a bit trickier than the others because some/all
+        # time_until_retry is a bit trickier than the others because some/all of the
-        # of the source values might be None
+        # source values might be None
         if all(r.time_until_retry is None for r in results):
             time_until_retry = None
         else:
@@ -156,9 +154,9 @@ class RateLimitResult:
 class RateLimitedAction:
     """Represents a particular action and the limits on its usage.
 
-    This class uses the redis-cell Redis module to implement a Generic Cell
+    This class uses the redis-cell Redis module to implement a Generic Cell Rate
-    Rate Algorithm (GCRA) for rate-limiting, which includes several desirable
+    Algorithm (GCRA) for rate-limiting, which includes several desirable characteristics
-    characteristics including a rolling time window and support for "bursts".
+    including a rolling time window and support for "bursts".
     """
 
     def __init__(
@@ -173,13 +171,12 @@ class RateLimitedAction:
     ) -> None:
         """Initialize the limits on a particular action.
 
-        The action will be limited to a maximum of `limit` calls over the time
+        The action will be limited to a maximum of `limit` calls over the time period
-        period specified in `period`. By default, up to half of the actions
+        specified in `period`. By default, up to half of the actions inside a period may
-        inside a period may be used in a "burst", in which no specific time
+        be used in a "burst", in which no specific time restrictions are applied.  This
-        restrictions are applied.  This behavior is controlled by the
+        behavior is controlled by the `max_burst` argument, which can range from 1 (no
-        `max_burst` argument, which can range from 1 (no burst allowed,
+        burst allowed, requests must wait at least `period / limit` time between them),
-        requests must wait at least `period / limit` time between them), up to
+        up to the same value as `limit` (the full limit may be used at any rate, but
-        the same value as `limit` (the full limit may be used at any rate, but
         never more than `limit` inside any given period).
         """
         # pylint: disable=too-many-arguments
@@ -202,8 +199,8 @@ class RateLimitedAction:
         self.by_user = by_user
         self.by_ip = by_ip
 
-        # if a redis connection wasn't specified, it will need to be
+        # if a redis connection wasn't specified, it will need to be initialized before
-        # initialized before any checks or resets for this action can be done
+        # any checks or resets for this action can be done
         self._redis = redis
 
     @property

tildes/tildes/lib/string.py

@@ -34,14 +34,14 @@ def convert_to_url_slug(original: str, max_length: int = 100) -> str:
     # url-encode the slug
     encoded_slug = quote(slug)
 
-    # if the slug's already short enough, just return without worrying about
+    # if the slug's already short enough, just return without worrying about how it will
-    # how it will need to be truncated
+    # need to be truncated
     if len(encoded_slug) <= max_length:
         return encoded_slug
 
-    # Truncating a url-encoded slug can be tricky if there are any multi-byte
+    # Truncating a url-encoded slug can be tricky if there are any multi-byte unicode
-    # unicode characters, since the %-encoded forms of them can be quite long.
+    # characters, since the %-encoded forms of them can be quite long. Check to see if
-    # Check to see if the slug looks like it might contain any of those.
+    # the slug looks like it might contain any of those.
     maybe_multi_bytes = bool(re.search("%..%", encoded_slug))
 
     # if that matched, we need to take a more complicated approach
@@ -56,9 +56,8 @@ def convert_to_url_slug(original: str, max_length: int = 100) -> str:
 
 def _truncate_multibyte_slug(original: str, max_length: int) -> str:
     """URL-encode and truncate a slug known to contain multi-byte chars."""
-    # instead of the normal method of truncating "backwards" from the end of
+    # instead of the normal method of truncating "backwards" from the end of the string,
-    # the string, build it up one encoded character at a time from the start
+    # build it up one encoded character at a time from the start until it's too long
-    # until it's too long
     encoded_slug = ""
     for character in original:
         encoded_char = quote(character)
@@ -69,14 +68,13 @@ def _truncate_multibyte_slug(original: str, max_length: int) -> str:
 
         encoded_slug += encoded_char
 
-    # Now we know that the string is made up of "whole" characters and is close
+    # Now we know that the string is made up of "whole" characters and is close to the
-    # to the maximum length. We'd still like to truncate it at an underscore if
+    # maximum length. We'd still like to truncate it at an underscore if possible, but
-    # possible, but some languages like Japanese and Chinese won't have many
+    # some languages like Japanese and Chinese won't have many (or any) underscores in
-    # (or any) underscores in the slug, and we could end up losing a lot of the
+    # the slug, and we could end up losing a lot of the characters. So try breaking it
-    # characters. So try breaking it at an underscore, but if it means more
+    # at an underscore, but if it means more than 30% of the slug gets cut off, just
-    # than 30% of the slug gets cut off, just leave it alone. This means that
+    # leave it alone. This means that some url slugs in other languages will end in
-    # some url slugs in other languages will end in partial words, but
+    # partial words, but determining the word edges is not simple.
-    # determining the word edges is not simple.
     acceptable_truncation = 0.7
 
     truncated_slug = truncate_string_at_char(encoded_slug, "_")
@@ -95,15 +93,13 @@ def truncate_string(
 ) -> str:
     """Truncate a string to be no longer than a specified length.
 
-    If `truncate_at_chars` is specified (as a string, one or more characters),
+    If `truncate_at_chars` is specified (as a string, one or more characters), the
-    the truncation will happen at the last occurrence of any of those chars
+    truncation will happen at the last occurrence of any of those chars inside the
-    inside the remaining string after it has been initially cut down to the
+    remaining string after it has been initially cut down to the desired length.
-    desired length.
 
-    `overflow_str` will be appended to the result, and its length is included
+    `overflow_str` will be appended to the result, and its length is included in the
-    in the final string length. So for example, if `overflow_str` has a length
+    final string length. So for example, if `overflow_str` has a length of 3 and the
-    of 3 and the target length is 10, at most 7 characters of the original
+    target length is 10, at most 7 characters of the original string will be kept.
-    string will be kept.
     """
     if overflow_str is None:
         overflow_str = ""
@@ -112,8 +108,8 @@ def truncate_string(
     if len(original) <= length:
         return original
 
-    # cut the string down to the max desired length (leaving space for the
+    # cut the string down to the max desired length (leaving space for the overflow
-    # overflow string if one is specified)
+    # string if one is specified)
     truncated = original[: length - len(overflow_str)]
 
     # if we don't want to truncate at particular characters, we're done
@@ -129,17 +125,17 @@ def truncate_string(
 def truncate_string_at_char(original: str, valid_chars: str) -> str:
     """Truncate a string at the last occurrence of a particular character.
 
-    Supports passing multiple valid characters (as a string) for `valid_chars`,
+    Supports passing multiple valid characters (as a string) for `valid_chars`, for
-    for example valid_chars='.?!' would truncate at the "right-most" occurrence
+    example valid_chars='.?!' would truncate at the "right-most" occurrence of any of
-    of any of those 3 characters in the string.
+    those 3 characters in the string.
     """
     # work backwards through the string until we find one of the chars we want
     for num_from_end, char in enumerate(reversed(original), start=1):
         if char in valid_chars:
             break
     else:
-        # the loop didn't break, so we looked through the entire string and
+        # the loop didn't break, so we looked through the entire string and didn't find
-        # didn't find any of the desired characters - can't do anything
+        # any of the desired characters - can't do anything
         return original
 
     # a truncation char was found, so -num_from_end is the position to stop at
@@ -150,14 +146,14 @@ def truncate_string_at_char(original: str, valid_chars: str) -> str:
 def simplify_string(original: str) -> str:
     """Sanitize a string for usage in places where we need a "simple" one.
 
-    This function is useful for sanitizing strings so that they're suitable to
+    This function is useful for sanitizing strings so that they're suitable to be used
-    be used in places like topic titles, message subjects, and so on.
+    in places like topic titles, message subjects, and so on.
 
     Strings processed by this function:
 
     * have unicode chars from the "separator" category replaced with spaces
-    * have unicode chars from the "other" category stripped out, except for
+    * have unicode chars from the "other" category stripped out, except for newlines,
-      newlines, which are replaced with spaces
+      which are replaced with spaces
     * have leading and trailing whitespace removed
     * have multiple consecutive spaces collapsed into a single space
     """
@@ -183,8 +179,8 @@ def _sanitize_characters(original: str) -> str:
             # "separator" chars - replace with a normal space
             final_characters.append(" ")
         elif category.startswith("C"):
-            # "other" chars (control, formatting, etc.) - filter them out
+            # "other" chars (control, formatting, etc.) - filter them out except for
-            # except for newlines, which are replaced with normal spaces
+            # newlines, which are replaced with normal spaces
             if char == "\n":
                 final_characters.append(" ")
         else:

tildes/tildes/metrics.py

@@ -1,7 +1,7 @@
 """Contains Prometheus metric objects and functions for instrumentation."""
 
-# The prometheus_client classes work in a pretty crazy way, need to disable
+# The prometheus_client classes work in a pretty crazy way, need to disable these pylint
-# these pylint checks to avoid errors
+# checks to avoid errors
 # pylint: disable=no-value-for-parameter,redundant-keyword-arg
 
 from typing import Callable

tildes/tildes/models/comment/comment.py

@@ -29,23 +29,23 @@ class Comment(DatabaseModel):
 
     Trigger behavior:
       Incoming:
-        - num_votes will be incremented and decremented by insertions and
+        - num_votes will be incremented and decremented by insertions and deletions in
-          deletions in comment_votes.
+          comment_votes.
       Outgoing:
-        - Inserting or deleting rows, or updating is_deleted/is_removed to
+        - Inserting or deleting rows, or updating is_deleted/is_removed to change
-          change visibility will increment or decrement num_comments
+          visibility will increment or decrement num_comments accordingly on the
-          accordingly on the relevant topic.
+          relevant topic.
-        - Inserting a row will increment num_comments on any topic_visit rows
+        - Inserting a row will increment num_comments on any topic_visit rows for the
-          for the comment's author and the relevant topic.
+          comment's author and the relevant topic.
-        - Inserting a new comment or updating is_deleted or is_removed will
+        - Inserting a new comment or updating is_deleted or is_removed will update
-          update last_activity_time on the relevant topic.
+          last_activity_time on the relevant topic.
         - Setting is_deleted or is_removed to true will delete any rows in
           comment_notifications related to the comment.
-        - Changing is_deleted or is_removed will adjust num_comments on all
+        - Changing is_deleted or is_removed will adjust num_comments on all topic_visit
-          topic_visit rows for the relevant topic, where the visit_time was
+          rows for the relevant topic, where the visit_time was after the time the
-          after the time the comment was originally posted.
+          comment was originally posted.
-        - Inserting a row or updating markdown will send a rabbitmq message
+        - Inserting a row or updating markdown will send a rabbitmq message for
-          for "comment.created" or "comment.edited" respectively.
+          "comment.created" or "comment.edited" respectively.
       Internal:
         - deleted_time will be set or unset when is_deleted is changed
         - removed_time will be set or unset when is_removed is changed

tildes/tildes/models/comment/comment_notification.py

@@ -22,11 +22,10 @@ class CommentNotification(DatabaseModel):
 
     Trigger behavior:
       Incoming:
-        - Rows will be deleted if the relevant comment has is_deleted set to
+        - Rows will be deleted if the relevant comment has is_deleted set to true.
-          true.
       Outgoing:
-        - Inserting, deleting, or updating is_unread will increment or
+        - Inserting, deleting, or updating is_unread will increment or decrement
-          decrement num_unread_notifications for the relevant user.
+          num_unread_notifications for the relevant user.
     """
 
     __tablename__ = "comment_notifications"
@@ -122,16 +121,14 @@ class CommentNotification(DatabaseModel):
     ) -> Tuple[List["CommentNotification"], List["CommentNotification"]]:
         """Filter new notifications for edited comments.
 
-        Protect against sending a notification for the same comment to
+        Protect against sending a notification for the same comment to the same user
-        the same user twice. Edits can sent notifications to users
+        twice. Edits can sent notifications to users now mentioned in the content, but
-        now mentioned in the content, but only if they weren't sent
+        only if they weren't sent a notification for that comment before.
-        a notification for that comment before.
 
-        This method returns a tuple of lists of this class. The first
+        This method returns a tuple of lists of this class. The first item is the
-        item is the notifications that were previously sent for this
+        notifications that were previously sent for this comment but need to be deleted
-        comment but need to be deleted (i.e. mentioned username was edited
+        (i.e. mentioned username was edited out of the comment), and the second item is
-        out of the comment), and the second item is the notifications
+        the notifications that need to be added, as they're new.
-        that need to be added, as they're new.
         """
         previous_notifications = (
             db_session.query(CommentNotification)

tildes/tildes/models/comment/comment_query.py

@@ -15,9 +15,8 @@ class CommentQuery(PaginatedQuery):
     def __init__(self, request: Request) -> None:
         """Initialize a CommentQuery for the request.
 
-        If the user is logged in, additional user-specific data will be fetched
+        If the user is logged in, additional user-specific data will be fetched along
-        along with the comments. For the moment, this is whether the user has
+        with the comments. For the moment, this is whether the user has voted on them.
-        voted on them.
         """
         super().__init__(Comment, request)
 

tildes/tildes/models/comment/comment_tree.py

@@ -14,8 +14,8 @@ class CommentTree:
 
     The Comment objects held by this class have additional attributes added:
         - `replies`: the list of all immediate children to that comment
-        - `has_visible_descendant`: whether the comment has any visible
+        - `has_visible_descendant`: whether the comment has any visible descendants (if
-            descendants (if not, it can be pruned from the tree)
+          not, it can be pruned from the tree)
     """
 
     def __init__(self, comments: Sequence[Comment], sort: CommentSortOption) -> None:
@@ -23,8 +23,8 @@ class CommentTree:
         self.tree: List[Comment] = []
         self.sort = sort
 
-        # sort the comments by date, since replies will always be posted later
+        # sort the comments by date, since replies will always be posted later this will
-        # this will ensure that parent comments are always processed first
+        # ensure that parent comments are always processed first
         self.comments = sorted(comments, key=lambda c: c.created_time)
 
         # if there aren't any comments, we can just bail out here
@@ -33,11 +33,10 @@ class CommentTree:
 
         self._build_tree()
 
-        # The method of building the tree already sorts it by posting time, so
+        # The method of building the tree already sorts it by posting time, so there's
-        # there's no need to sort again if that's the desired sorting. Note
+        # no need to sort again if that's the desired sorting. Note also that because
-        # also that because _sort_tree() uses sorted() which is a stable sort,
+        # _sort_tree() uses sorted() which is a stable sort, this means that the
-        # this means that the "secondary sort" will always be by posting time
+        # "secondary sort" will always be by posting time as well.
-        # as well.
         if sort != CommentSortOption.POSTED:
             with self._sorting_histogram().time():
                 self.tree = self._sort_tree(self.tree, self.sort)
@@ -75,9 +74,9 @@ class CommentTree:
     def _sort_tree(tree: List[Comment], sort: CommentSortOption) -> List[Comment]:
         """Sort the tree by the desired ordering (recursively).
 
-        Because Python's sorted() function is stable, the ordering of any
+        Because Python's sorted() function is stable, the ordering of any comments that
-        comments that compare equal on the sorting method will be the same as
+        compare equal on the sorting method will be the same as the order that they were
-        the order that they were originally in when passed to this function.
+        originally in when passed to this function.
         """
         if sort == CommentSortOption.NEWEST:
             tree = sorted(tree, key=lambda c: c.created_time, reverse=True)

tildes/tildes/models/comment/comment_vote.py

@@ -17,8 +17,8 @@ class CommentVote(DatabaseModel):
 
     Trigger behavior:
       Outgoing:
-        - Inserting or deleting a row will increment or decrement the num_votes
+        - Inserting or deleting a row will increment or decrement the num_votes column
-          column for the relevant comment.
+          for the relevant comment.
     """
 
     __tablename__ = "comment_votes"

tildes/tildes/models/database_model.py

@@ -51,8 +51,8 @@ class DatabaseModelBase:
         if not isinstance(other, self.__class__):
             return NotImplemented
 
-        # loop over all the columns in the primary key - if any don't match,
+        # loop over all the columns in the primary key - if any don't match, return
-        # return False, otherwise return True if we get through all of them
+        # False, otherwise return True if we get through all of them
         for column in self.__table__.primary_key:
             if getattr(self, column.name) != getattr(other, column.name):
                 return False
@@ -62,8 +62,8 @@ class DatabaseModelBase:
     def __hash__(self) -> int:
         """Return the hash value of the model.
 
-        This is implemented by mixing together the hash values of the primary
+        This is implemented by mixing together the hash values of the primary key
-        key columns used in __eq__, as recommended in the Python documentation.
+        columns used in __eq__, as recommended in the Python documentation.
         """
         primary_key_values = tuple(
             getattr(self, column.name) for column in self.__table__.primary_key
@@ -84,28 +84,25 @@ class DatabaseModelBase:
     def _validate_new_value(self, attribute: str, value: Any) -> Any:
         """Validate the new value for a column.
 
-        This function will be attached to the SQLAlchemy ORM attribute event
+        This function will be attached to the SQLAlchemy ORM attribute event for "set"
-        for "set" and will be called whenever a new value is assigned to any of
+        and will be called whenever a new value is assigned to any of a model's column
-        a model's column attributes. It works by deserializing/loading the new
+        attributes. It works by deserializing/loading the new value through the
-        value through the marshmallow schema associated with the model class
+        marshmallow schema associated with the model class (by its `schema` class
-        (by its `schema` class attribute).
+        attribute).
 
-        The deserialization process can modify the value if desired (for
+        The deserialization process can modify the value if desired (for sanitization),
-        sanitization), or raise an exception which will prevent the assignment
+        or raise an exception which will prevent the assignment from happening at all.
-        from happening at all.
 
-        Note that if the schema does not have a Field defined for the column,
+        Note that if the schema does not have a Field defined for the column, or the
-        or the Field is declared dump_only, no validation/sanitization will be
+        Field is declared dump_only, no validation/sanitization will be applied.
-        applied.
         """
         if not self.schema_class:
             return value
 
-        # This is a bit "magic", but simplifies the interaction between this
+        # This is a bit "magic", but simplifies the interaction between this validation
-        # validation and SQLAlchemy hybrid properties. If the attribute being
+        # and SQLAlchemy hybrid properties. If the attribute being set starts with an
-        # set starts with an underscore, assume that it's due to being set up
+        # underscore, assume that it's due to being set up as a hybrid property, and
-        # as a hybrid property, and remove the underscore prefix when looking
+        # remove the underscore prefix when looking for a field to validate against.
-        # for a field to validate against.
         if attribute.startswith("_"):
             attribute = attribute[1:]
 

tildes/tildes/models/group/group.py

@@ -17,8 +17,8 @@ class Group(DatabaseModel):
 
     Trigger behavior:
       Incoming:
-        - num_subscriptions will be incremented and decremented by insertions
+        - num_subscriptions will be incremented and decremented by insertions and
-          and deletions in group_subscriptions.
+          deletions in group_subscriptions.
     """
 
     schema_class = GroupSchema
@@ -45,9 +45,9 @@ class Group(DatabaseModel):
         Boolean, nullable=False, server_default="false"
     )
 
-    # Create a GiST index on path as well as the btree one that will be created
+    # Create a GiST index on path as well as the btree one that will be created by the
-    # by the index=True/unique=True keyword args to Column above. The GiST
+    # index=True/unique=True keyword args to Column above. The GiST index supports
-    # index supports additional operators for ltree queries: @>, <@, @, ~, ?
+    # additional operators for ltree queries: @>, <@, @, ~, ?
     __table_args__ = (Index("ix_groups_path_gist", path, postgresql_using="gist"),)
 
     def __repr__(self) -> str:

tildes/tildes/models/group/group_query.py

@@ -15,9 +15,8 @@ class GroupQuery(ModelQuery):
     def __init__(self, request: Request) -> None:
         """Initialize a GroupQuery for the request.
 
-        If the user is logged in, additional user-specific data will be fetched
+        If the user is logged in, additional user-specific data will be fetched along
-        along with the groups. For the moment, this is whether the user is
+        with the groups. For the moment, this is whether the user is subscribed to them.
-        subscribed to them.
         """
         super().__init__(Group, request)
 

tildes/tildes/models/group/group_subscription.py

@@ -17,8 +17,8 @@ class GroupSubscription(DatabaseModel):
 
     Trigger behavior:
       Outgoing:
-        - Inserting or deleting a row will increment or decrement the
+        - Inserting or deleting a row will increment or decrement the num_subscriptions
-          num_subscriptions column for the relevant group.
+          column for the relevant group.
     """
 
     __tablename__ = "group_subscriptions"

tildes/tildes/models/log/log.py

@@ -74,9 +74,8 @@ class Log(DatabaseModel, BaseLog):
     ) -> None:
         """Create a new log entry.
 
-        User and IP address info is extracted from the Request object.
+        User and IP address info is extracted from the Request object. `info` is an
-        `info` is an optional dict of arbitrary data that will be stored in
+        optional dict of arbitrary data that will be stored in JSON form.
-        JSON form.
         """
         self.user = request.user
         self.event_type = event_type

tildes/tildes/models/message/message.py

@@ -1,13 +1,12 @@
 """Contains the MessageConversation and MessageReply classes.
 
-Note the difference between these two classes - MessageConversation represents
+Note the difference between these two classes - MessageConversation represents both the
-both the overall conversation and the initial message in a particular message
+overall conversation and the initial message in a particular message
-conversation/thread. Subsequent replies (if any) inside that same conversation
+conversation/thread. Subsequent replies (if any) inside that same conversation are
-are represented by MessageReply.
+represented by MessageReply.
-
+This might feel a bit unusual since it splits "all messages" across two tables/classes,
-This might feel a bit unusual since it splits "all messages" across two
+but it simplifies a lot of things when organizing them into threads.
-tables/classes, but it simplifies a lot of things when organizing them into
-threads.
 """
 
 from datetime import datetime
@@ -44,13 +43,13 @@ class MessageConversation(DatabaseModel):
 
     Trigger behavior:
       Incoming:
-        - num_replies, last_reply_time, and unread_user_ids are updated when a
+        - num_replies, last_reply_time, and unread_user_ids are updated when a new
-          new message_replies row is inserted for the conversation.
+          message_replies row is inserted for the conversation.
-        - num_replies and last_reply_time will be updated if a message_replies
+        - num_replies and last_reply_time will be updated if a message_replies row is
-          row is deleted.
+          deleted.
       Outgoing:
-        - Inserting or updating unread_user_ids will update num_unread_messages
+        - Inserting or updating unread_user_ids will update num_unread_messages for all
-          for all relevant users.
+          relevant users.
     """
 
     schema_class = MessageConversationSchema
@@ -95,9 +94,9 @@ class MessageConversation(DatabaseModel):
         "MessageReply", order_by="MessageReply.created_time"
     )
 
-    # Create a GIN index on the unread_user_ids column using the gin__int_ops
+    # Create a GIN index on the unread_user_ids column using the gin__int_ops operator
-    # operator class supplied by the intarray module. This should be the best
+    # class supplied by the intarray module. This should be the best index for "array
-    # index for "array contains" queries.
+    # contains" queries.
     __table_args__ = (
         Index(
             "ix_message_conversations_unread_user_ids_gin",
@@ -151,8 +150,8 @@ class MessageConversation(DatabaseModel):
     def other_user(self, viewer: User) -> User:
         """Return the conversation's other user from viewer's perspective.
 
-        That is, if the viewer is the sender, this will be the recipient, and
+        That is, if the viewer is the sender, this will be the recipient, and vice
-        vice versa.
+        versa.
         """
         if not self.is_participant(viewer):
             raise ValueError("User is not a participant in this conversation.")
@@ -172,8 +171,8 @@ class MessageConversation(DatabaseModel):
     def mark_unread_for_user(self, user: User) -> None:
         """Mark the conversation unread for the specified user.
 
-        Uses the postgresql intarray union operator `|`, so there's no need to
+        Uses the postgresql intarray union operator `|`, so there's no need to worry
-        worry about duplicate values, race conditions, etc.
+        about duplicate values, race conditions, etc.
         """
         if not self.is_participant(user):
             raise ValueError("User is not a participant in this conversation.")
@@ -184,9 +183,9 @@ class MessageConversation(DatabaseModel):
     def mark_read_for_user(self, user: User) -> None:
         """Mark the conversation read for the specified user.
 
-        Uses the postgresql intarray "remove element from array" operation, so
+        Uses the postgresql intarray "remove element from array" operation, so there's
-        there's no need to worry about whether the value is present or not,
+        no need to worry about whether the value is present or not, race conditions,
-        race conditions, etc.
+        etc.
         """
         if not self.is_participant(user):
             raise ValueError("User is not a participant in this conversation.")
@@ -202,8 +201,8 @@ class MessageReply(DatabaseModel):
 
     Trigger behavior:
       Outgoing:
-        - Inserting will update num_replies, last_reply_time, and
+        - Inserting will update num_replies, last_reply_time, and unread_user_ids for
-          unread_user_ids for the relevant conversation.
+          the relevant conversation.
         - Deleting will update num_replies and last_reply_time for the relevant
           conversation.
     """

tildes/tildes/models/model_query.py

@@ -30,8 +30,8 @@ class ModelQuery(Query):
     def __iter__(self) -> Iterator[ModelType]:
         """Iterate over the (processed) results of the query.
 
-        SQLAlchemy goes through __iter__ to execute the query and return the
+        SQLAlchemy goes through __iter__ to execute the query and return the results, so
-        results, so adding processing here should cover all the possibilities.
+        adding processing here should cover all the possibilities.
         """
         results = super().__iter__()
         return iter([self._process_result(result) for result in results])
@@ -44,10 +44,10 @@ class ModelQuery(Query):
         """Finalize the query before it's executed."""
         # pylint: disable=protected-access
 
-        # Assertions are disabled to allow these functions to add more filters
+        # Assertions are disabled to allow these functions to add more filters even
-        # even though .limit() or .offset() may have already been called. This
+        # though .limit() or .offset() may have already been called. This is potentially
-        # is potentially dangerous, but should be fine with the existing
+        # dangerous, but should be fine with the existing straightforward usage
-        # straightforward usage patterns.
+        # patterns.
         return (
             self.enable_assertions(False)
             ._attach_extra_data()
@@ -58,9 +58,9 @@ class ModelQuery(Query):
     def _before_compile_listener(self) -> "ModelQuery":
         """Do any final adjustments to the query before it's compiled.
 
-        Note that this method cannot be overridden by subclasses because of
+        Note that this method cannot be overridden by subclasses because of the way it
-        the way it is subscribed to the event. Subclasses should override the
+        is subscribed to the event. Subclasses should override the _finalize() method
-        _finalize() method instead if necessary.
+        instead if necessary.
         """
         return self._finalize()
 
@@ -81,13 +81,13 @@ class ModelQuery(Query):
     def lock_based_on_request_method(self) -> "ModelQuery":
         """Lock the rows if request method implies it's needed (generative).
 
-        Applying this function to a query will cause the database to acquire
+        Applying this function to a query will cause the database to acquire a row-level
-        a row-level FOR UPDATE lock on any rows the query retrieves. This is
+        FOR UPDATE lock on any rows the query retrieves. This is only done if the
-        only done if the request method is DELETE, PATCH, or PUT, which all
+        request method is DELETE, PATCH, or PUT, which all imply that the item(s) being
-        imply that the item(s) being fetched are going to be modified.
+        fetched are going to be modified.
 
-        Note that POST is specifically not included, because the item being
+        Note that POST is specifically not included, because the item being POSTed to is
-        POSTed to is not usually modified in a "dangerous" way as a result.
+        not usually modified in a "dangerous" way as a result.
         """
         if self.request.method in {"DELETE", "PATCH", "PUT"}:
             return self.with_for_update(of=self.model_cls)
@@ -109,8 +109,8 @@ class ModelQuery(Query):
     def join_all_relationships(self) -> "ModelQuery":
         """Eagerly join all lazy relationships (generative).
 
-        This is useful for being able to load an item "fully" in a single
+        This is useful for being able to load an item "fully" in a single query and
-        query and avoid needing to make additional queries for related items.
+        avoid needing to make additional queries for related items.
         """
         # pylint: disable=no-member
         self = self.options(Load(self.model_cls).joinedload("*"))
@@ -129,8 +129,8 @@ class ModelQuery(Query):
         return result
 
 
-# add a listener so the _finalize() function will be called automatically just
+# add a listener so the _finalize() function will be called automatically just before
-# before the query executes
+# the query executes
 event.listen(
     ModelQuery,
     "before_compile",

tildes/tildes/models/pagination.py

@@ -56,22 +56,20 @@ class PaginatedQuery(ModelQuery):
     def is_reversed(self) -> bool:
         """Return whether the query is operating "in reverse".
 
-        This is a bit confusing. When moving "forward" through pages, items
+        This is a bit confusing. When moving "forward" through pages, items will be
-        will be queried in the same order that they are displayed. For example,
+        queried in the same order that they are displayed. For example, when displaying
-        when displaying the newest topics, the query is simply for "newest N
+        the newest topics, the query is simply for "newest N topics" (where N is the
-        topics" (where N is the number of items per page), with an optional
+        number of items per page), with an optional "after topic X" clause. Either way,
-        "after topic X" clause. Either way, the first result from the query
+        the first result from the query will have the highest created_time, and should
-        will have the highest created_time, and should be the first item
+        be the first item displayed.
-        displayed.
+        However, things work differently when you are paging "backwards". Since this is
-
+        done by looking before a specific item, the query needs to fetch items in the
-        However, things work differently when you are paging "backwards". Since
+        opposite order of how they will be displayed. For the "newest" sort example,
-        this is done by looking before a specific item, the query needs to
+        when paging backwards you need to query for "*oldest* N items before topic X",
-        fetch items in the opposite order of how they will be displayed. For
+        so the query ordering is the exact opposite of the desired display order. The
-        the "newest" sort example, when paging backwards you need to query for
+        first result from the query will have the *lowest* created_time, so should be
-        "*oldest* N items before topic X", so the query ordering is the exact
+        the last item displayed. Because of this, the results need to be reversed.
-        opposite of the desired display order. The first result from the query
-        will have the *lowest* created_time, so should be the last item
-        displayed. Because of this, the results need to be reversed.
         """
         return bool(self.before_id)
 
@@ -100,20 +98,20 @@ class PaginatedQuery(ModelQuery):
 
         query = self
 
-        # determine the ID of the "anchor item" that we're using as an upper or
+        # determine the ID of the "anchor item" that we're using as an upper or lower
-        # lower bound, and which type of bound it is
+        # bound, and which type of bound it is
         if self.after_id:
             anchor_id = self.after_id
 
-            # since we're looking for other items "after" the anchor item, it
+            # since we're looking for other items "after" the anchor item, it will act
-            # will act as an upper bound when the sort order is descending,
+            # as an upper bound when the sort order is descending, otherwise it's a
-            # otherwise it's a lower bound
+            # lower bound
             is_anchor_upper_bound = self.sort_desc
         elif self.before_id:
             anchor_id = self.before_id
 
-            # opposite of "after" behavior - when looking "before" the anchor
+            # opposite of "after" behavior - when looking "before" the anchor item, it's
-            # item, it's an upper bound if the sort order is *ascending*
+            # an upper bound if the sort order is *ascending*
             is_anchor_upper_bound = not self.sort_desc
 
         # create a subquery to get comparison values for the anchor item
@@ -136,8 +134,8 @@ class PaginatedQuery(ModelQuery):
         """Finalize the query before execution."""
         query = super()._finalize()
 
-        # if the query is reversed, we need to sort in the opposite dir
+        # if the query is reversed, we need to sort in the opposite dir (basically
-        # (basically self.sort_desc XOR self.is_reversed)
+        # self.sort_desc XOR self.is_reversed)
         desc = self.sort_desc
         if self.is_reversed:
             desc = not desc
@@ -167,18 +165,18 @@ class PaginatedResults:
         """Fetch results from a PaginatedQuery."""
         self.per_page = per_page
 
-        # if the query had `before` or `after` restrictions, there must be a
+        # if the query had `before` or `after` restrictions, there must be a page in
-        # page in that direction (it's where we came from)
+        # that direction (it's where we came from)
         self.has_next_page = bool(query.before_id)
         self.has_prev_page = bool(query.after_id)
 
-        # fetch the results - try to get one more than we're actually going to
+        # fetch the results - try to get one more than we're actually going to display,
-        # display, so that we know if there's another page
+        # so that we know if there's another page
         self.results = query.limit(per_page + 1).all()
 
-        # if we managed to get one more item than the page size, there's
+        # if we managed to get one more item than the page size, there's another page in
-        # another page in the same direction that we're going - set the
+        # the same direction that we're going - set the relevant attr and remove the
-        # relevant attr and remove the extra item so it's not displayed
+        # extra item so it's not displayed
         if len(self.results) > per_page:
             if query.is_reversed:
                 self.results = self.results[1:]
@@ -187,8 +185,8 @@ class PaginatedResults:
                 self.has_next_page = True
                 self.results = self.results[:-1]
 
-        # if the query came back empty for some reason, we won't be able to
+        # if the query came back empty for some reason, we won't be able to have
-        # have next/prev pages since there are no items to base them on
+        # next/prev pages since there are no items to base them on
         if not self.results:
             self.has_next_page = False
             self.has_prev_page = False

tildes/tildes/models/topic/topic.py

@@ -46,15 +46,15 @@ class Topic(DatabaseModel):
 
     Trigger behavior:
       Incoming:
-        - num_votes will be incremented and decremented by insertions and
+        - num_votes will be incremented and decremented by insertions and deletions in
-          deletions in topic_votes.
+          topic_votes.
-        - num_comments will be incremented and decremented by insertions,
+        - num_comments will be incremented and decremented by insertions, deletions, and
-          deletions, and updates to is_deleted in comments.
-        - last_activity_time will be updated by insertions, deletions, and
           updates to is_deleted in comments.
+        - last_activity_time will be updated by insertions, deletions, and updates to
+          is_deleted in comments.
       Outgoing:
-        - Inserting a row or updating markdown will send a rabbitmq message
+        - Inserting a row or updating markdown will send a rabbitmq message for
-          for "topic.created" or "topic.edited" respectively.
+          "topic.created" or "topic.edited" respectively.
       Internal:
         - deleted_time will be set when is_deleted is set to true
     """
@@ -317,8 +317,8 @@ class Topic(DatabaseModel):
         if not self.is_link_type or not self.link:
             raise ValueError("Non-link topics do not have a domain")
 
-        # get the domain from the content metadata if possible, but fall back
+        # get the domain from the content metadata if possible, but fall back to just
-        # to just parsing it from the link if it's not present
+        # parsing it from the link if it's not present
         return self.get_content_metadata("domain") or get_domain_from_url(self.link)
 
     @property
@@ -329,8 +329,8 @@ class Topic(DatabaseModel):
     def get_content_metadata(self, key: str) -> Any:
         """Get a piece of content metadata "safely".
 
-        Will return None if the topic has no metadata defined, if this key
+        Will return None if the topic has no metadata defined, if this key doesn't exist
-        doesn't exist in the metadata, etc.
+        in the metadata, etc.
         """
         if not isinstance(self.content_metadata, dict):
             return None

tildes/tildes/models/topic/topic_query.py

@@ -21,10 +21,10 @@ class TopicQuery(PaginatedQuery):
     def __init__(self, request: Request) -> None:
         """Initialize a TopicQuery for the request.
 
-        If the user is logged in, additional user-specific data will be fetched
+        If the user is logged in, additional user-specific data will be fetched along
-        along with the topics. For the moment, this is whether the user has
+        with the topics. For the moment, this is whether the user has voted on the
-        voted on the topics, and data related to their last visit - what time
+        topics, and data related to their last visit - what time they last visited, and
-        they last visited, and how many new comments have been posted since.
+        how many new comments have been posted since.
         """
         super().__init__(Topic, request)
 
@@ -118,9 +118,9 @@ class TopicQuery(PaginatedQuery):
 
     def inside_time_period(self, period: SimpleHoursPeriod) -> "TopicQuery":
         """Restrict the topics to inside a time period (generative)."""
-        # if the time period is too long, this will crash by creating a
+        # if the time period is too long, this will crash by creating a datetime outside
-        # datetime outside the valid range - catch that and just don't filter
+        # the valid range - catch that and just don't filter by time period at all if
-        # by time period at all if the range is that large
+        # the range is that large
         try:
             start_time = utc_now() - period.timedelta
         except OverflowError:
@@ -130,9 +130,9 @@ class TopicQuery(PaginatedQuery):
 
     def has_tag(self, tag: Ltree) -> "TopicQuery":
         """Restrict the topics to ones with a specific tag (generative)."""
-        # casting tag to string really shouldn't be necessary, but some kind of
+        # casting tag to string really shouldn't be necessary, but some kind of strange
-        # strange interaction seems to be happening with the ArrayOfLtree
+        # interaction seems to be happening with the ArrayOfLtree class, this will need
-        # class, this will need some investigation
+        # some investigation
         tag = str(tag)
 
         # pylint: disable=protected-access

tildes/tildes/models/topic/topic_visit.py

@@ -16,16 +16,16 @@ from .topic import Topic
 class TopicVisit(DatabaseModel):
     """Model for a user's visit to a topic.
 
-    New visits should not be created through __init__(), but by executing the
+    New visits should not be created through __init__(), but by executing the statement
-    statement returned by the `generate_insert_statement` method. This will
+    returned by the `generate_insert_statement` method. This will take advantage of
-    take advantage of postgresql's ability to update any existing visit.
+    postgresql's ability to update any existing visit.
 
     Trigger behavior:
       Incoming:
-        - num_comments will be incremented for the author's topic visit when
+        - num_comments will be incremented for the author's topic visit when they post a
-          they post a comment in that topic.
+          comment in that topic.
-        - num_comments will be decremented when a comment is deleted, for all
+        - num_comments will be decremented when a comment is deleted, for all visits to
-          visits to the topic that were after it was posted.
+          the topic that were after it was posted.
     """
 
     __tablename__ = "topic_visits"

tildes/tildes/models/topic/topic_vote.py

@@ -17,8 +17,8 @@ class TopicVote(DatabaseModel):
 
     Trigger behavior:
       Outgoing:
-        - Inserting or deleting a row will increment or decrement the num_votes
+        - Inserting or deleting a row will increment or decrement the num_votes column
-          column for the relevant topic.
+          for the relevant topic.
     """
 
     __tablename__ = "topic_votes"

tildes/tildes/models/user/user.py

@@ -39,12 +39,10 @@ class User(DatabaseModel):
 
     Trigger behavior:
       Incoming:
-        - num_unread_notifications will be incremented and decremented by
+        - num_unread_notifications will be incremented and decremented by insertions,
-          insertions, deletions, and updates to is_unread in
+          deletions, and updates to is_unread in comment_notifications.
-          comment_notifications.
+        - num_unread_messages will be incremented and decremented by insertions,
-        - num_unread_messages will be incremented and decremented by
+          deletions, and updates to unread_user_ids in message_conversations.
-          insertions, deletions, and updates to unread_user_ids in
-          message_conversations.
     """
 
     schema_class = UserSchema
@@ -142,8 +140,8 @@ class User(DatabaseModel):
 
     @password.setter
     def password(self, value: str) -> None:
-        # need to do manual validation since some password checks depend on
+        # need to do manual validation since some password checks depend on checking the
-        # checking the username at the same time (for similarity)
+        # username at the same time (for similarity)
         self.schema.validate({"username": self.username, "password": value})
 
         self.password_hash = hash_string(value)

tildes/tildes/models/user/user_invite_code.py

@@ -50,8 +50,8 @@ class UserInviteCode(DatabaseModel):
     def __init__(self, user: User) -> None:
         """Create a new (random) invite code owned by the user.
 
-        Note that uniqueness is not confirmed here, so there is the potential
+        Note that uniqueness is not confirmed here, so there is the potential to create
-        to create duplicate codes (which will fail to commit to the database).
+        duplicate codes (which will fail to commit to the database).
         """
         self.user_id = user.user_id
 
@@ -64,8 +64,8 @@ class UserInviteCode(DatabaseModel):
         # codes are stored in uppercase
         code = code.upper()
 
-        # remove any characters that aren't in the code alphabet (allows
+        # remove any characters that aren't in the code alphabet (allows dashes, spaces,
-        # dashes, spaces, etc. to be used to make the codes more readable)
+        # etc. to be used to make the codes more readable)
         code = "".join(letter for letter in code if letter in cls.ALPHABET)
 
         if len(code) > cls.LENGTH:

tildes/tildes/resources/__init__.py

@@ -8,9 +8,9 @@ from tildes.models import DatabaseModel, ModelQuery
 
 def get_resource(request: Request, base_query: ModelQuery) -> DatabaseModel:
     """Prepare and execute base query from a root factory, returning result."""
-    # While the site is private, we don't want to leak information about which
+    # While the site is private, we don't want to leak information about which usernames
-    # usernames or groups exist. So we should just always raise a 403 before
+    # or groups exist. So we should just always raise a 403 before doing a lookup and
-    # doing a lookup and potentially raising a 404.
+    # potentially raising a 404.
     if not request.user:
         raise HTTPForbidden
 

tildes/tildes/resources/comment.py

@@ -28,8 +28,8 @@ def notification_by_comment_id36(
 ) -> CommentNotification:
     """Get a comment notification specified by {comment_id36} in the route.
 
-    Looks up a comment notification for the logged-in user with the
+    Looks up a comment notification for the logged-in user with the {comment_id36}
-    {comment_id36} specified in the route.
+    specified in the route.
     """
     if not request.user:
         raise HTTPForbidden

tildes/tildes/resources/group.py

@@ -16,10 +16,10 @@ from tildes.schemas.group import GroupSchema
 )
 def group_by_path(request: Request, path: str) -> Group:
     """Get a group specified by {group_path} in the route (or 404)."""
-    # If loading the specified group path into the GroupSchema changed it, do a
+    # If loading the specified group path into the GroupSchema changed it, do a 301
-    # 301 redirect to the resulting group path. This will happen in cases like
+    # redirect to the resulting group path. This will happen in cases like the original
-    # the original url including capital letters in the group path, where we
+    # url including capital letters in the group path, where we want to redirect to the
-    # want to redirect to the proper all-lowercase path instead.
+    # proper all-lowercase path instead.
     if path != request.matchdict["group_path"]:
         request.matchdict["group_path"] = path
         proper_url = request.route_url(request.matched_route.name, **request.matchdict)

tildes/tildes/resources/topic.py

@@ -22,8 +22,8 @@ def topic_by_id36(request: Request, topic_id36: str) -> Topic:
 
     topic = get_resource(request, query)
 
-    # if there's also a group specified in the route, check that it's the same
+    # if there's also a group specified in the route, check that it's the same group as
-    # group as the topic was posted in, otherwise redirect to correct group
+    # the topic was posted in, otherwise redirect to correct group
     if "group_path" in request.matchdict:
         path_from_route = request.matchdict["group_path"].lower()
         if path_from_route != topic.group.path:

tildes/tildes/routes.py

@@ -149,9 +149,9 @@ def add_intercooler_routes(config: Configurator) -> None:
 class LoggedInFactory:
     """Simple class to use as `factory` to restrict routes to logged-in users.
 
-    This class can be used when a route should only be accessible to logged-in
+    This class can be used when a route should only be accessible to logged-in users but
-    users but doesn't already have another factory that would handle that by
+    doesn't already have another factory that would handle that by checking access to a
-    checking access to a specific resource (such as a topic or message).
+    specific resource (such as a topic or message).
     """
 
     __acl__ = ((Allow, Authenticated, "view"),)

tildes/tildes/schemas/__init__.py

@@ -2,14 +2,14 @@
 
 These schemas are currently being used for several purposes:
 
-  - Validation of data for models, such as checking the lengths of strings,
+  - Validation of data for models, such as checking the lengths of strings, ensuring
-    ensuring that they match a particular regex pattern, etc. Specific errors
+    that they match a particular regex pattern, etc. Specific errors can be generated
-    can be generated for any data that is invalid.
+    for any data that is invalid.
 
-  - Similarly, the webargs library uses the schemas to validate pieces of data
+  - Similarly, the webargs library uses the schemas to validate pieces of data coming in
-    coming in via urls, POST data, etc. It can produce errors if the data is
+    via urls, POST data, etc. It can produce errors if the data is not valid for the
-    not valid for the purpose it's intended for.
+    purpose it's intended for.
 
-  - Serialization of data, which the Pyramid JSON renderer uses to produce
+  - Serialization of data, which the Pyramid JSON renderer uses to produce data for the
-    data for the JSON API endpoints.
+    JSON API endpoints.
 """

tildes/tildes/schemas/fields.py

@@ -108,8 +108,8 @@ class SimpleString(Field):
     These strings should generally not contain any special formatting (such as
     markdown), and have problematic whitespace/unicode/etc. removed.
 
-    See the simplify_string() function for full details of how these strings
+    See the simplify_string() function for full details of how these strings are
-    are processed and sanitized.
+    processed and sanitized.
 
     """
 

tildes/tildes/schemas/topic.py

@@ -68,11 +68,11 @@ class TopicSchema(Schema):
     def validate_tags(self, value: typing.List[sqlalchemy_utils.Ltree]) -> None:
         """Validate the tags field, raising an error if an issue exists.
 
-        Note that tags are validated by ensuring that each tag would be a valid
+        Note that tags are validated by ensuring that each tag would be a valid group
-        group path. This is definitely mixing concerns, but it's deliberate in
+        path. This is definitely mixing concerns, but it's deliberate in this case. It
-        this case. It will allow for some interesting possibilities by ensuring
+        will allow for some interesting possibilities by ensuring naming "compatibility"
-        naming "compatibility" between groups and tags. For example, a popular
+        between groups and tags. For example, a popular tag in a group could be
-        tag in a group could be converted into a sub-group easily.
+        converted into a sub-group easily.
         """
         group_schema = GroupSchema(partial=True)
         for tag in value:

tildes/tildes/schemas/user.py

@@ -16,9 +16,9 @@ USERNAME_MAX_LENGTH = 20
 # Valid username regex, encodes the following:
 #   - must start with a number or letter
 #   - must end with a number or letter
-#   - the middle can contain numbers, letters, underscores and dashes, but no
+#   - the middle can contain numbers, letters, underscores and dashes, but no more than
-#     more than one underscore/dash consecutively (this includes both "_-" and
+#     one underscore/dash consecutively (this includes both "_-" and "-_" sequences
-#     "-_" sequences being invalid)
+#     being invalid)
 # Note: this regex does not contain any length checks, must be done separately
 # fmt: off
 USERNAME_VALID_REGEX = re.compile(
@@ -110,9 +110,9 @@ class UserSchema(Schema):
 def is_valid_username(username: str) -> bool:
     """Return whether the username is valid or not.
 
-    Simple convenience wrapper that uses the schema to validate a username,
+    Simple convenience wrapper that uses the schema to validate a username, useful in
-    useful in cases where a simple valid/invalid result is needed without
+    cases where a simple valid/invalid result is needed without worrying about the
-    worrying about the specific reason for invalidity.
+    specific reason for invalidity.
     """
     schema = UserSchema(partial=True)
     try:

tildes/tildes/views/__init__.py

@@ -8,6 +8,6 @@ from pyramid.response import Response
 IC_NOOP = Response(status_int=200)
 IC_NOOP_404 = Response(status_int=404)
 
-# Because of the above, in order to deliberately cause Intercooler to replace
+# Because of the above, in order to deliberately cause Intercooler to replace an element
-# an element with whitespace, the response needs to contain at least two spaces
+# with whitespace, the response needs to contain at least two spaces
 IC_EMPTY = Response("  ")

tildes/tildes/views/api/web/comment.py

@@ -22,14 +22,13 @@ from tildes.views.decorators import ic_view_config
 def _increment_topic_comments_seen(request: Request, comment: Comment) -> None:
     """Increment the number of comments in a topic the user has viewed.
 
-    If the user has the "track comment visits" feature enabled, we want to
+    If the user has the "track comment visits" feature enabled, we want to increment the
-    increment the number of comments they've seen in the thread that the
+    number of comments they've seen in the thread that the comment came from, so that
-    comment came from, so that they don't *both* get a notification as well as
+    they don't *both* get a notification as well as have the thread highlight with "(1
-    have the thread highlight with "(1 new)". This should only happen if their
+    new)". This should only happen if their last visit was before the comment was
-    last visit was before the comment was posted, however.  Below, this is
+    posted, however.  Below, this is implemented as a INSERT ... ON CONFLICT DO UPDATE
-    implemented as a INSERT ... ON CONFLICT DO UPDATE so that it will insert
+    so that it will insert a new topic visit with 1 comment if they didn't previously
-    a new topic visit with 1 comment if they didn't previously have one at
+    have one at all.
-    all.
     """
     if request.user.track_comment_visits:
         statement = (
@@ -190,8 +189,8 @@ def put_vote_comment(request: Request) -> dict:
     request.db_session.add(new_vote)
 
     try:
-        # manually flush before attempting to commit, to avoid having all
+        # manually flush before attempting to commit, to avoid having all objects
-        # objects detached from the session in case of an error
+        # detached from the session in case of an error
         request.db_session.flush()
         request.tm.commit()
     except IntegrityError:
@@ -254,8 +253,8 @@ def put_tag_comment(request: Request, name: CommentTagOption) -> Response:
     request.db_session.add(tag)
 
     try:
-        # manually flush before attempting to commit, to avoid having all
+        # manually flush before attempting to commit, to avoid having all objects
-        # objects detached from the session in case of an error
+        # detached from the session in case of an error
         request.db_session.flush()
         request.tm.commit()
     except FlushError:
@@ -309,9 +308,9 @@ def delete_tag_comment(request: Request, name: CommentTagOption) -> Response:
 def put_mark_comments_read(request: Request, mark_all_previous: bool) -> Response:
     """Mark comment(s) read, clearing notifications.
 
-    The "main" notification (request.context) will always be marked read, and
+    The "main" notification (request.context) will always be marked read, and if the
-    if the query param mark_all_previous is Truthy, all notifications prior to
+    query param mark_all_previous is Truthy, all notifications prior to that one will be
-    that one will be marked read as well.
+    marked read as well.
     """
     notification = request.context
 

tildes/tildes/views/api/web/exceptions.py

@@ -91,11 +91,11 @@ def httptoomanyrequests(request: Request) -> Response:
 def httpfound(request: Request) -> Response:
     """Convert an HTTPFound to a 200 with the header for a redirect.
 
-    Intercooler won't handle a 302 response as a "full" redirect, and will just
+    Intercooler won't handle a 302 response as a "full" redirect, and will just load the
-    load the content of the destination page into the target element, the same
+    content of the destination page into the target element, the same as any other
-    as any other response. However, it has support for a special X-IC-Redirect
+    response. However, it has support for a special X-IC-Redirect header, which allows
-    header, which allows the response to trigger a client-side redirect. This
+    the response to trigger a client-side redirect. This exception view will convert a
-    exception view will convert a 302 into a 200 with that header so it works
+    302 into a 200 with that header so it works as a redirect for both standard requests
-    as a redirect for both standard requests as well as Intercooler ones.
+    as well as Intercooler ones.
     """
     return Response(headers={"X-IC-Redirect": request.exception.location})

tildes/tildes/views/api/web/group.py

@@ -32,8 +32,8 @@ def put_subscribe_group(request: Request) -> dict:
     request.db_session.add(new_subscription)
 
     try:
-        # manually flush before attempting to commit, to avoid having all
+        # manually flush before attempting to commit, to avoid having all objects
-        # objects detached from the session in case of an error
+        # detached from the session in case of an error
         request.db_session.flush()
         request.tm.commit()
     except IntegrityError:

tildes/tildes/views/api/web/topic.py

@@ -87,8 +87,8 @@ def put_topic_vote(request: Request) -> Response:
     request.db_session.add(new_vote)
 
     try:
-        # manually flush before attempting to commit, to avoid having all
+        # manually flush before attempting to commit, to avoid having all objects
-        # objects detached from the session in case of an error
+        # detached from the session in case of an error
         request.db_session.flush()
         request.tm.commit()
     except IntegrityError:

tildes/tildes/views/api/web/user.py

@@ -66,10 +66,10 @@ def patch_change_email_address(
     """Change the user's email address (and descriptive note)."""
     user = request.context
 
-    # If the user already has an email address set, we need to retain the
+    # If the user already has an email address set, we need to retain the previous hash
-    # previous hash and description in the log. Otherwise, if an account is
+    # and description in the log. Otherwise, if an account is compromised and the
-    # compromised and the attacker changes the email address, we'd have no way
+    # attacker changes the email address, we'd have no way to support recovery for the
-    # to support recovery for the owner.
+    # owner.
     log_info = None
     if user.email_address_hash:
         log_info = {
@@ -155,8 +155,8 @@ def get_invite_code(request: Request) -> dict:
     # obtain a lock to prevent concurrent requests generating multiple codes
     request.obtain_lock("generate_invite_code", user.user_id)
 
-    # it's possible to randomly generate an existing code, so we'll retry
+    # it's possible to randomly generate an existing code, so we'll retry until we
-    # until we create a new one (will practically always be the first try)
+    # create a new one (will practically always be the first try)
     while True:
         savepoint = request.tm.savepoint()
 
@@ -169,9 +169,9 @@ def get_invite_code(request: Request) -> dict:
         except IntegrityError:
             savepoint.rollback()
 
-    # doing an atomic decrement on request.user.invite_codes_remaining is going
+    # doing an atomic decrement on request.user.invite_codes_remaining is going to make
-    # to make it unusable as an integer in the template, so store the expected
+    # it unusable as an integer in the template, so store the expected value after the
-    # value after the decrement first, to be able to use that instead
+    # decrement first, to be able to use that instead
     num_remaining = request.user.invite_codes_remaining - 1
     request.user.invite_codes_remaining = User.invite_codes_remaining - 1
 

tildes/tildes/views/decorators.py

@@ -28,9 +28,8 @@ def rate_limit_view(action_name: str) -> Callable:
     Needs to be used with the name of the rate-limited action, such as:
     @rate_limit_view('register')
 
-    If the ratelimit check comes back with the action being blocked, a 429
+    If the ratelimit check comes back with the action being blocked, a 429 response with
-    response with appropriate headers will be raised instead of calling the
+    appropriate headers will be raised instead of calling the decorated view.
-    decorated view.
     """
 
     def decorator(func: Callable) -> Callable:
@@ -51,10 +50,9 @@ def rate_limit_view(action_name: str) -> Callable:
 def not_logged_in(func: Callable) -> Callable:
     """Decorate a view function to prevent access by logged-in users.
 
-    If a logged-in user attempts to access a view decorated by this function,
+    If a logged-in user attempts to access a view decorated by this function, they will
-    they will be redirected to the home page instead. This is useful for views
+    be redirected to the home page instead. This is useful for views such as the login
-    such as the login page, registration page, etc. which only logged-out users
+    page, registration page, etc. which only logged-out users should be accessing.
-    should be accessing.
     """
 
     def wrapper(request: Request, **kwargs: Any) -> Any:

tildes/tildes/views/message.py

@@ -22,9 +22,9 @@ def get_new_message_form(request: Request) -> dict:
 @view_config(route_name="messages", renderer="messages.jinja2")
 def get_user_messages(request: Request) -> dict:
     """Show the logged-in user's message conversations."""
-    # select conversations where either the user is the recipient, or they
+    # select conversations where either the user is the recipient, or they were the
-    # were the sender and there is at least one reply (don't need to show
+    # sender and there is at least one reply (don't need to show conversations the user
-    # conversations the user started but haven't been replied to)
+    # started but haven't been replied to)
     conversations = (
         request.query(MessageConversation)
         .filter(
@@ -63,8 +63,8 @@ def get_user_unread_messages(request: Request) -> dict:
 @view_config(route_name="messages_sent", renderer="messages_sent.jinja2")
 def get_user_sent_messages(request: Request) -> dict:
     """Show the logged-in user's sent message conversations."""
-    # select conversations where either the user was the sender, or they
+    # select conversations where either the user was the sender, or they were the
-    # were the recipient and there is at least one reply
+    # recipient and there is at least one reply
     conversations = (
         request.query(MessageConversation)
         .filter(

tildes/tildes/views/metrics.py

@@ -13,10 +13,10 @@ def get_metrics(request: Request) -> str:
     multiprocess.MultiProcessCollector(registry)
     data = generate_latest(registry)
 
-    # When Prometheus accesses this page it will always create a new session.
+    # When Prometheus accesses this page it will always create a new session.  This
-    # This session is useless and will never be used again, so we can just
+    # session is useless and will never be used again, so we can just invalidate it to
-    # invalidate it to cause it to be deleted from storage. It would be even
+    # cause it to be deleted from storage. It would be even better to find a way to not
-    # better to find a way to not create it in the first place.
+    # create it in the first place.
     request.session.invalidate()
 
     return data.decode("utf-8")

tildes/tildes/views/notifications.py

@@ -22,8 +22,8 @@ def get_user_unread_notifications(request: Request) -> dict:
         .all()
     )
 
-    # if the user has the "automatically mark notifications as read" setting
+    # if the user has the "automatically mark notifications as read" setting enabled,
-    # enabled, mark all their notifications as read
+    # mark all their notifications as read
     if request.user.auto_mark_notifications_read:
         for notification in notifications:
             notification.is_unread = False

tildes/tildes/views/register.py

@@ -32,8 +32,8 @@ def get_register(request: Request) -> dict:
 def user_schema_check_breaches(request: Request) -> UserSchema:
     """Return a UserSchema that will check the password against breaches.
 
-    It would probably be good to generalize this function at some point,
+    It would probably be good to generalize this function at some point, probably
-    probably similar to:
+    similar to:
     http://webargs.readthedocs.io/en/latest/advanced.html#reducing-boilerplate
     """
     # pylint: disable=unused-argument
@@ -67,8 +67,8 @@ def post_register(
     if password != password_confirm:
         raise HTTPUnprocessableEntity("Password and confirmation do not match.")
 
-    # attempt to fetch and lock the row for the specified invite code (lock
+    # attempt to fetch and lock the row for the specified invite code (lock prevents
-    # prevents concurrent requests from using the same invite code)
+    # concurrent requests from using the same invite code)
     lookup_code = UserInviteCode.prepare_code_for_lookup(invite_code)
     code_row = (
         request.query(UserInviteCode)
@@ -95,8 +95,8 @@ def post_register(
     except IntegrityError:
         raise HTTPUnprocessableEntity("That username has already been registered.")
 
-    # the flush above will generate the new user's ID, so use that to update
+    # the flush above will generate the new user's ID, so use that to update the invite
-    # the invite code with info about the user that registered with it
+    # code with info about the user that registered with it
     code_row.invitee_id = user.user_id
 
     # subscribe the new user to all groups except ~test

tildes/tildes/views/topic.py

@@ -47,8 +47,8 @@ def post_group_topics(
             group=request.context, author=request.user, title=title, link=link
         )
 
-        # if they specified both a link and markdown, use the markdown to post
+        # if they specified both a link and markdown, use the markdown to post an
-        # an initial comment on the topic
+        # initial comment on the topic
         if markdown:
             new_comment = Comment(
                 topic=new_topic, author=request.user, markdown=markdown
@@ -139,8 +139,8 @@ def get_group_topics(
 
     period_options = [SimpleHoursPeriod(hours) for hours in (1, 12, 24, 72)]
 
-    # add the current period to the bottom of the dropdown if it's not one of
+    # add the current period to the bottom of the dropdown if it's not one of the
-    # the "standard" ones
+    # "standard" ones
     if period and period not in period_options:
         period_options.append(period)
 
@@ -177,8 +177,8 @@ def get_topic(request: Request, comment_order: CommentSortOption) -> dict:
     """View a single topic."""
     topic = request.context
 
-    # deleted and removed comments need to be included since they're necessary
+    # deleted and removed comments need to be included since they're necessary for
-    # for building the tree if they have replies
+    # building the tree if they have replies
     comments = (
         request.query(Comment)
         .include_deleted()
@@ -263,8 +263,8 @@ def _get_default_settings(request: Request, order: Any) -> DefaultSettings:
     else:
         default_order = TopicSortOption.ACTIVITY
 
-    # the default period depends on what the order is, so we need to see if
+    # the default period depends on what the order is, so we need to see if we're going
-    # we're going to end up using the default order here as well
+    # to end up using the default order here as well
     if order is missing:
         order = default_order
 
@@ -275,8 +275,8 @@ def _get_default_settings(request: Request, order: Any) -> DefaultSettings:
         user_default = request.user.home_default_period
         default_period = ShortTimePeriod().deserialize(user_default)
     else:
-        # Overall default periods, if the user doesn't have either a
+        # Overall default periods, if the user doesn't have either a group-specific or a
-        # group-specific or a home default set up:
+        # home default set up:
         #   * "all time" if sorting by new
         #   * "all time" if sorting by activity and inside a group
         #   * "3 days" if sorting by activity and on home page

tildes/tildes/views/user.py

@@ -20,9 +20,9 @@ def _get_user_recent_activity(
 ) -> List[Union[Comment, Topic]]:
     page_size = 20
 
-    # Since we don't know how many comments or topics will be needed to make
+    # Since we don't know how many comments or topics will be needed to make up a page,
-    # up a page, we'll fetch the full page size of both types, merge them,
+    # we'll fetch the full page size of both types, merge them, and then trim down to
-    # and then trim down to the size afterwards
+    # the size afterwards
     query = (
         request.query(Comment)
         .filter(Comment.user == user)