chore: Audit code for missing type hints and redundant comments

Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>

Author: gendeux

src/rendezqueue/cli.py

@@ -2,9 +2,10 @@
 import sys
 import threading
 import time
+from typing import List
 from .client import RendezqueueClient
 
-def main():
+def main() -> None:
     parser = argparse.ArgumentParser(description="Rendezqueue Client")
     parser.add_argument("--url", required=True, help="URL of the Rendezqueue server")
     parser.add_argument("--key", required=True, help="Session key")
@@ -14,7 +15,7 @@ def main():
 
     stop_event = threading.Event()
 
-    def on_data(values):
+    def on_data(values: List[bytes]) -> None:
         for v in values:
             try:
                 # Try to decode as utf-8, fallback to repr or raw bytes
@@ -23,11 +24,8 @@ def on_data(values):
                 print(v)
         sys.stdout.flush()
 
-    def on_error(e):
+    def on_error(e: Exception) -> None:
         print(f"Error: {e}", file=sys.stderr)
-        # Maybe we should exit on error?
-        # But connection errors might be transient.
-        # If it's a fatal error, we might want to stop.
 
     client = RendezqueueClient(
         url=args.url,
@@ -41,7 +39,7 @@ def on_error(e):
         client.start()
 
         # Reader thread
-        def read_stdin():
+        def read_stdin() -> None:
             try:
                 for line in sys.stdin:
                     if stop_event.is_set():
@@ -53,11 +51,6 @@ def read_stdin():
                 # stdin closed or error
                 pass
             finally:
-                # If stdin closes, we might want to stop eventually?
-                # Or keep running to receive?
-                # A typical chat client exits when stdin closes.
-                # However, for rendezqueue, we might want to wait for a response.
-                # So we don't stop automatically on EOF.
                 pass
 
         input_thread = threading.Thread(target=read_stdin, daemon=True)

src/rendezqueue/client.py

@@ -7,12 +7,12 @@
 import time
 import urllib.request
 import urllib.error
-from typing import Callable, Optional, List
+from typing import Callable, Optional, List, Dict, Any
 
 logger = logging.getLogger(__name__)
 
 class RendezqueueClient:
-    def __init__(self, url: str, key: str, hue: str, on_data: Optional[Callable[[List[bytes]], None]] = None, on_error: Optional[Callable[[Exception], None]] = None, poll_interval_ms: int = 2000):
+    def __init__(self, url: str, key: str, hue: str, on_data: Optional[Callable[[List[bytes]], None]] = None, on_error: Optional[Callable[[Exception], None]] = None, poll_interval_ms: int = 2000) -> None:
         self.url = url
         self.key = key
         self.hue = hue
@@ -23,44 +23,42 @@ def __init__(self, url: str, key: str, hue: str, on_data: Optional[Callable[[Lis
         self.sid_counter = 1
         self.sid = self._generate_sid()
         self.offset = 0
-        self.outgoing_queue = []
+        self.outgoing_queue: List[bytes] = []
         self.lock = threading.Lock()
 
         self.is_polling = False
         self.is_stopped = True
-        self.poll_thread = None
+        self.poll_thread: Optional[threading.Thread] = None
 
-    def _generate_sid(self):
+    def _generate_sid(self) -> str:
         random_part = ''.join(random.choices(string.ascii_lowercase + string.digits, k=7))
         return f"{self.hue}-{self.sid_counter}-{random_part}"
 
-    def start(self):
+    def start(self) -> None:
         if not self.is_stopped:
             return
         self.is_stopped = False
         self.poll_thread = threading.Thread(target=self._poll_loop, daemon=True)
         self.poll_thread.start()
 
-    def stop(self):
+    def stop(self) -> None:
         if self.is_stopped:
             return
         self.is_stopped = True
-        # We don't join here because stop might be called from within a callback
-        # which might be running in the poll thread (if on_data is synchronous)
 
-    def send(self, value):
+    def send(self, value: Any) -> None:
         if isinstance(value, str):
             value = value.encode('utf-8')
         with self.lock:
             self.outgoing_queue.append(value)
 
-    def _start_new_session(self):
+    def _start_new_session(self) -> None:
         with self.lock:
             self.sid_counter += 1
             self.sid = self._generate_sid()
             self.offset = 0
 
-    def _poll_loop(self):
+    def _poll_loop(self) -> None:
         # Initial poll
         self._poll()
         while not self.is_stopped:
@@ -69,7 +67,7 @@ def _poll_loop(self):
                 break
             self._poll()
 
-    def _poll(self):
+    def _poll(self) -> None:
         if self.is_polling:
             return
         self.is_polling = True
@@ -145,7 +143,7 @@ def _poll(self):
         finally:
             self.is_polling = False
 
-    def _decode_response(self, msg):
+    def _decode_response(self, msg: Dict[str, Any]) -> Dict[str, Any]:
         b64_flags = msg.get('b64', 0)
         if b64_flags & 4:
             msg['key'] = self._b64decode_padded(msg['key']).decode('utf-8')
@@ -155,7 +153,7 @@ def _decode_response(self, msg):
             msg['values'] = [self._b64decode_padded(v) for v in msg['values']]
         return msg
 
-    def _b64decode_padded(self, s):
+    def _b64decode_padded(self, s: str) -> bytes:
         missing_padding = len(s) % 4
         if missing_padding:
             s += '=' * (4 - missing_padding)

src/rendezqueue/impl.py

@@ -58,7 +58,7 @@ def inplace_decode_tryswap_message(msg: Dict[str, Any]) -> str:
         if msg['b64'] & 1:
             msg['values'] = [atob(v) for v in msg['values']]
     except Exception:
-        return "b64_decode_error" # Custom error, JS might just crash or throw
+        return "b64_decode_error"
 
     return ""
 
@@ -82,19 +82,17 @@ def inplace_encode_tryswap_message(msg: Dict[str, Any]) -> None:
         del msg['ttl']
     if 'offset' in msg and msg['offset'] == 0:
         del msg['offset']
-    # If values is None, remove it (TrySwapResponse handles optional)
     if 'values' in msg and msg['values'] is None:
         del msg['values']
 
 class RendezqueueImpl:
-    def __init__(self):
+    def __init__(self) -> None:
         self.swapstore = SwapStore()
 
     def tryswap(self, msg: Any, now_ms: Optional[float] = None) -> Union[int, TrySwapResponse]:
         if not msg:
             return 400
 
-        # We assume msg is a dict here, validated by inplace_decode... but we should be careful
         if not isinstance(msg, dict):
             return 400
 
@@ -114,47 +112,33 @@ def tryswap(self, msg: Any, now_ms: Optional[float] = None) -> Union[int, TrySwa
 
         if now_ms is None:
             now_ms = time.time() * 1000
-            if now_ms == 0: # unlikely but matches JS
+            if now_ms == 0:
                 return 500
-            now_ms = int(now_ms) # JS uses Math.floor
+            now_ms = int(now_ms)
 
         result = self.swapstore.tryswap(
             key, sid, offset, values,
             now_ms, msg.get('ttl', 0),
         )
 
-        if not isinstance(result, int):
-            # It's a TrySwapResponse object. We need to convert it to dict for encoding
-            # or wrap it.
-            # But the caller expects to encode it.
-            # Python dataclass to dict:
-            # We can use vars(result) or similar
-            # But we need to preserve b64 from request
-            pass
-
         return result
 
     def tryswap_string(self, request_text: str) -> Union[int, str]:
-        msg = None
+        msg: Optional[Dict[str, Any]] = None
         try:
             msg = json.loads(request_text)
             e = inplace_decode_tryswap_message(msg)
             if e:
-                # In JS: throw e
-                # Here we can return error code or message?
-                # The JS code catches 'e' and logs it, then sets msg = null.
-                # So if decoding fails, msg is null.
                 raise Exception(e)
-        except Exception as e:
-            # print(e) # Optional logging
+        except Exception:
             msg = None
 
         result = self.tryswap(msg)
         if isinstance(result, int):
             return result
 
         # Convert dataclass to dict
-        res_dict = {
+        res_dict: Dict[str, Any] = {
             'key': result.key,
             'sid': result.sid,
             'offset': result.offset,

src/rendezqueue/server.py

@@ -3,16 +3,16 @@
 import argparse
 from urllib.parse import urlparse
 from rendezqueue.impl import RendezqueueImpl
-from typing import cast
+from typing import cast, Tuple, Any
 
 class RendezqueueServer(socketserver.ThreadingMixIn, http.server.HTTPServer):
-    def __init__(self, server_address, RequestHandlerClass, impl, http_path="/"):
+    def __init__(self, server_address: Tuple[str, int], RequestHandlerClass: Any, impl: RendezqueueImpl, http_path: str = "/") -> None:
         self.impl = impl
         self.http_path = http_path
         super().__init__(server_address, RequestHandlerClass)
 
 class RendezqueueHandler(http.server.BaseHTTPRequestHandler):
-    def do_OPTIONS(self):
+    def do_OPTIONS(self) -> None:
         self.send_response(204)
         if "Access-Control-Request-Headers" in self.headers:
             self.send_header("Access-Control-Allow-Headers", self.headers["Access-Control-Request-Headers"])
@@ -22,8 +22,7 @@ def do_OPTIONS(self):
             self.send_header("Access-Control-Allow-Origin", self.headers["Origin"])
         self.end_headers()
 
-    def do_POST(self):
-        # Cast self.server to RendezqueueServer to access custom attributes
+    def do_POST(self) -> None:
         server = cast(RendezqueueServer, self.server)
 
         parsed_path = urlparse(self.path)
@@ -53,14 +52,13 @@ def do_POST(self):
             self.end_headers()
             self.wfile.write(result.encode("utf-8"))
 
-def run():
+def run() -> None:
     parser = argparse.ArgumentParser(description="Rendezqueue Server")
     parser.add_argument("--http_host", default="127.0.0.1", help="Host to bind to")
     parser.add_argument("--http_port", type=int, default=0, help="Port to bind to (0 for random)")
     parser.add_argument("--http_path", default="/", help="Path for RPC")
     parser.add_argument("--port_filepath", help="File to write the port number to")
 
-    # Allow unknown args to be ignored (e.g. for testing or future flags)
     args, unknown = parser.parse_known_args()
 
     impl = RendezqueueImpl()
@@ -72,8 +70,6 @@ def run():
         http_path=args.http_path
     )
 
-    # server_address might be (host, port) or more depending on family
-    # For AF_INET it is (host, port)
     host, port = server.server_address[:2]
     print(f"Server running at http://{host}:{port}{args.http_path}")
 

src/rendezqueue/swapstore.py

@@ -24,15 +24,14 @@ class TrySwapResponse:
     ttl: Optional[int] = None
 
 class SwapStore:
-    def __init__(self):
+    def __init__(self) -> None:
         # key -> UnmatchedOffer
         self.unmatched_offer_map: Dict[str, UnmatchedOffer] = {}
         # key -> sid -> SwappedAnswer
         self.swapped_answer_multimap: Dict[str, Dict[str, SwappedAnswer]] = {}
         self.ttl = MAX_TTL_SECONDS
 
     def expire_swapped_answers(self, key: str, now_ms: float) -> bool:
-        """Presumably, data was exchanged."""
         answer_map = self.swapped_answer_multimap.get(key)
         if not answer_map:
             return True
@@ -52,7 +51,6 @@ def expire_swapped_answers(self, key: str, now_ms: float) -> bool:
         return False
 
     def expire_unmatched_offers(self, now_ms: float) -> None:
-        """No takers."""
         expiring_offers = []
         # Python dicts maintain insertion order, allowing this logic to work like JS Map
         for key, v in self.unmatched_offer_map.items():
@@ -68,16 +66,11 @@ def expire_unmatched_offers(self, now_ms: float) -> None:
     @staticmethod
     def matches_original(original_values: List[str], offset: int, values: List[str]) -> bool:
         if len(original_values) < offset:
-            return False  # Too far ahead. Out of place.
+            return False
         if len(original_values) > offset + len(values):
-            return False  # Too short. Out of place.
+            return False
 
         original_slice = original_values[offset:]
-        # Compare element by element
-        # JS logic: original_slice.every((v, i) => values[i] == v)
-        # This implies values must match original_slice where they overlap.
-        # Since we already checked that original_values isn't longer than offset+values,
-        # we know values covers original_slice entirely (or extends it).
 
         for i, v in enumerate(original_slice):
             if values[i] != v:
@@ -111,10 +104,9 @@ def tryswap(self, key: str, sid: str, offset: int, values: List[str], now_ms: fl
         offer = self.unmatched_offer_map.get(key)
         if offer and offer.expiry_ms == 0:
             del self.unmatched_offer_map[key]
-            offer = None  # Fall through to next case.
-            self.expire_swapped_answers(key, now_ms)  # Ensure stuff would expire.
+            offer = None
+            self.expire_swapped_answers(key, now_ms)
 
-        # We'll need to make an offer.
         if not offer:
             if offset != 0:
                 return 404
@@ -131,15 +123,9 @@ def tryswap(self, key: str, sid: str, offset: int, values: List[str], now_ms: fl
                 ttl=ttl,
             )
 
-        # Still no match? Might as well reset expiry.
         if offer.sid == sid:
             original_values = offer.values or []
             if SwapStore.matches_original(original_values, offset, values):
-                # Update offer with new values appended
-                # Note: original code slices then concats.
-                # original_values.slice(0, offset).concat(values)
-                # But match checks that values matches suffix of original_values starting at offset.
-                # So if matched, we are extending the offer.
                 new_values = original_values[:offset] + values
 
                 self.unmatched_offer_map[key] = UnmatchedOffer(
@@ -156,7 +142,6 @@ def tryswap(self, key: str, sid: str, offset: int, values: List[str], now_ms: fl
             return 404
 
         if offset != 0:
-            # Invalid offset. We had no existing data!
             return 404
 
         if answer_map is None:
@@ -169,20 +154,13 @@ def tryswap(self, key: str, sid: str, offset: int, values: List[str], now_ms: fl
             expiry_ms=now_ms + ttl * 1000,
         )
 
-        # offer.sid is guaranteed to exist and offer.sid != sid here.
         if offer.sid:
             answer_map[offer.sid] = SwappedAnswer(
                 original_values=offer.values or [],
                 values=values,
                 expiry_ms=now_ms + ttl * 1000,
             )
 
-        # Remove the unmatched offer as it is now matched
-        # But instead of deleting, we mark it as expired (expiry_ms=0)
-        # so it can be cleaned up later or overwritten?
-        # JS code:
-        # this.unmatched_offer_map.delete(key);
-        # this.unmatched_offer_map.set(key, { expiry_ms: 0 });
         del self.unmatched_offer_map[key]
         self.unmatched_offer_map[key] = UnmatchedOffer(expiry_ms=0)