src/telegram_backup.py (3)

1509-1533: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Hash direct downloads too.

When deduplicate_media is false, this path still writes content_hash=None. That leaves scheduled-backup rows inconsistent with the listener flow and prevents later hash-based reuse if dedup is turned back on. Compute the hash from file_path before building media_data.

Proposed fix

             else:
                 # No deduplication - download directly to chat directory
                 if not os.path.exists(file_path):
                     actual_path = await self.client.download_media(message, file_path)
                     if actual_path and isinstance(actual_path, str):
                         file_path = actual_path
                     logger.debug(f"Downloaded media: {file_name}")
 
                 # Update file_size with actual size from disk
                 if os.path.exists(file_path):
                     file_size = os.path.getsize(file_path)
+                    content_hash = compute_file_hash(file_path)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/telegram_backup.py` around lines 1509 - 1533, When deduplicate_media is
False the code leaves content_hash as None; after the download branch that may
update file_path (the await self.client.download_media(...) block) compute the
content hash from the downloaded file_path (only if the file exists) and set
content_hash before constructing media_data so scheduled-backup rows include the
actual hash; update the logic around file_path/content_hash prior to the
media_data dict creation (referencing file_path, content_hash, download_media /
self.client.download_media, and media_data).

---

1477-1501: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Don't move a reused shared file out of _shared.

After a content-hash hit, shared_file_path may point at an older canonical blob. If os.symlink() then fails, shutil.move(shared_file_path, file_path) removes that canonical file from _shared, which can leave existing symlinks dangling and makes later dedup miss. Copy in this branch, or only move files created by this call. The same fallback exists in src/listener.py.

Proposed fix

+                        reused_existing_shared = False
                         content_hash = compute_file_hash(shared_file_path)
                         if content_hash and hasattr(self, "db"):
                             existing = await self.db.find_media_by_content_hash(content_hash)
                             if existing and existing.get("file_name"):
                                 existing_shared = os.path.join(shared_dir, existing["file_name"])
                                 if os.path.exists(existing_shared) and existing_shared != shared_file_path:
                                     os.remove(shared_file_path)
                                     shared_file_path = existing_shared
+                                    reused_existing_shared = True
                                     logger.debug(
                                         f"Content-hash dedup: {file_name} matches existing {existing['file_name']}"
                                     )
@@
                         except OSError as e:
                             # Symlink not supported (e.g., Windows) - move file to chat dir instead
                             logger.warning(f"Symlink not supported, using direct path: {e}")
                             import shutil
 
-                            shutil.move(shared_file_path, file_path)
+                            if reused_existing_shared:
+                                shutil.copy2(shared_file_path, file_path)
+                            else:
+                                shutil.move(shared_file_path, file_path)

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/telegram_backup.py` around lines 1477 - 1501, The fallback branch
currently moves shared_file_path into file_path which deletes canonical blobs
when a content-hash hit reused an existing file; change the fallback to copy
instead of move when shared_file_path points into the shared_dir or when the
file was resolved via db.find_media_by_content_hash (i.e. after
compute_file_hash and existing lookup), so that existing canonical files are
preserved; only perform shutil.move if the file was created by this call
(detectable by a local-temp path or a flag you set when writing the file),
otherwise use shutil.copy2(shared_file_path, file_path) and preserve
permissions/mtime.

---

101-104: ⚠️ Potential issue | 🔴 Critical

Fix Python 3 exception syntax at line 103.

except ValueError, TypeError: is invalid Python 3 syntax and prevents the module from importing. Change to tuple syntax: except (ValueError, TypeError):.

Proposed fix

-    except ValueError, TypeError:
+    except (ValueError, TypeError):
         log_threshold_seconds = 10

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/telegram_backup.py` around lines 101 - 104, The except clause in the
log_threshold_seconds parsing block uses Python 2 syntax (`except ValueError,
TypeError:`) which breaks imports; update the exception handling in that
try/except around int(os.getenv("FLOOD_WAIT_LOG_THRESHOLD", "10")) so it catches
both exceptions using tuple syntax `except (ValueError, TypeError):`, leaving
the fallback assignment to log_threshold_seconds = 10 unchanged.

src/listener.py (1)

910-923: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Persist file_name for listener-created media rows.

find_media_by_content_hash() rebuilds the canonical _shared path from Media.file_name. These inserts leave file_name null, so a listener-downloaded row can be selected for a hash hit and then treated as unusable, which makes later backup/listener dedup miss. Return file_name from _download_media() and store it here.

Proposed fix

-    async def _download_media(self, message, chat_id: int) -> tuple[str, str | None] | None:
+    async def _download_media(self, message, chat_id: int) -> tuple[str, str, str | None] | None:
@@
-            return f"{self.config.media_path}/{chat_id}/{file_name}", file_content_hash
+            return f"{self.config.media_path}/{chat_id}/{file_name}", file_name, file_content_hash
@@
-                            if download_result:
-                                media_path, content_hash = download_result
+                            if download_result:
+                                media_path, file_name, content_hash = download_result
                                 # Create media record (FK to messages now satisfied)
                                 media_id = f"{chat_id}_{message.id}_{media_type}"
                                 await self.db.insert_media(
                                     {
                                         "id": media_id,
                                         "message_id": message.id,
                                         "chat_id": chat_id,
                                         "type": media_type,
+                                        "file_name": file_name,
                                         "file_path": media_path,
                                         "content_hash": content_hash,
                                         "downloaded": True,
                                         "download_date": datetime.utcnow(),
                                     }
                                 )

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/listener.py` around lines 910 - 923, The listener inserts downloaded
media rows without setting Media.file_name which breaks
find_media_by_content_hash; update _download_media to return file_name (in
addition to media_path and content_hash), change the unpacking of
download_result in listener.py to media_path, content_hash, file_name, and
include "file_name": file_name in the dict passed to self.db.insert_media (keep
media_id, message_id, chat_id, type, file_path, content_hash, downloaded as-is)
so listener-created rows have file_name populated for canonical path
reconstruction.

tests/test_listener_extended.py (1)

1612-1638: ⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Add content_hash assertion to complete the coverage for this feature.

The mock now returns (path, content_hash) but the test only asserts on file_path and downloaded. If the listener silently drops the hash when calling insert_media, this test won't catch it — and persisting content_hash is the core objective of this PR.

✅ Proposed assertion addition

 media_data = db.insert_media.call_args[0][0]
 assert media_data["file_path"] == "/tmp/media/-100/photo.jpg"
 assert media_data["downloaded"] is True
+assert media_data.get("content_hash") is None  # None passed from mock tuple's second element

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@tests/test_listener_extended.py` around lines 1612 - 1638, The test mocks
listener._download_media to return ("/tmp/media/-100/photo.jpg",
"expected_hash") but never asserts that the listener forwards the content_hash
into db.insert_media; update the test after calling handler(event) to check
media_data = db.insert_media.call_args[0][0] and add an assertion that
media_data["content_hash"] == "expected_hash" (ensuring the mocked content_hash
value is used), leaving the existing assertions for file_path and downloaded
intact and referencing listener._download_media, handler, and db.insert_media to
locate the relevant lines.