show_transfer_stats and show_disable_debugging_reminder were each defined twice; the later definitions silently shadowed the earlier ones. The live show_transfer_stats(self, stats, operation) took only two args, but TransferManager passed a third (deduplicator), raising TypeError inside the Tk after() callback — so the transfer-stats window never appeared after folder transfers (it failed silently with a console traceback).
The shadowed copies were also dead code: they indexed stats["bytes_saved"]/["total_files"]/etc. — keys the *_with_dedup TODO stubs in adb_manager.py never produce (they return {"message": ...}), so they'd have KeyError'd on real data.
Removed the dead, shadowed copies of both methods
Dropped the now-unused deduplicator argument at the caller
Annotated the surviving methods with -> None
Behavior change: the transfer-stats window will now actually appear after folder transfers, rendering Message: <…> (generic dump). The richer structured summary depends on implementing the *_with_dedup TODO and is out of scope here.
2. Python 3.14 tooling/doc sync
The project already declares requires-python = ">=3.13, <3.15" and CI tests ['3.13','3.14'], but black/docs ignored 3.14.
black target-version → ['py313', 'py314']
CLAUDE.md → "Requires Python 3.13 or 3.14 (>=3.13, <3.15)"
mypy intentionally kept at the 3.13 floor (oldest supported runtime)
Verification
✅ 209 tests pass on Python 3.14.5
✅ GUI launches clean (python -m src.main, no traceback)
✅ Black clean; flake8 F811 cleared; mypy no-redef for both methods gone
✅ Each method now defined once; old 3-arg call correctly rejected
## Summary
Two fixes, both confined to existing-state cleanup (no new features).
### 1. Duplicate `DialogManager` methods → runtime `TypeError`
`show_transfer_stats` and `show_disable_debugging_reminder` were each **defined twice**; the later definitions silently shadowed the earlier ones. The live `show_transfer_stats(self, stats, operation)` took only two args, but `TransferManager` passed a third (`deduplicator`), raising `TypeError` inside the Tk `after()` callback — so the transfer-stats window **never appeared after folder transfers** (it failed silently with a console traceback).
The shadowed copies were also dead code: they indexed `stats["bytes_saved"]`/`["total_files"]`/etc. — keys the `*_with_dedup` TODO stubs in `adb_manager.py` never produce (they return `{"message": ...}`), so they'd have `KeyError`'d on real data.
- Removed the dead, shadowed copies of both methods
- Dropped the now-unused `deduplicator` argument at the caller
- Annotated the surviving methods with `-> None`
**Behavior change:** the transfer-stats window will now actually appear after folder transfers, rendering `Message: <…>` (generic dump). The richer structured summary depends on implementing the `*_with_dedup` TODO and is out of scope here.
### 2. Python 3.14 tooling/doc sync
The project already declares `requires-python = ">=3.13, <3.15"` and CI tests `['3.13','3.14']`, but black/docs ignored 3.14.
- `black target-version` → `['py313', 'py314']`
- `CLAUDE.md` → "Requires Python 3.13 or 3.14 (>=3.13, <3.15)"
- `mypy` intentionally kept at the **3.13 floor** (oldest supported runtime)
## Verification
- ✅ 209 tests pass on Python 3.14.5
- ✅ GUI launches clean (`python -m src.main`, no traceback)
- ✅ Black clean; flake8 F811 cleared; mypy `no-redef` for both methods gone
- ✅ Each method now defined once; old 3-arg call correctly rejected
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Code Review — PR 48: Fix duplicate DialogManager methods and make tooling Python 3.14-aware
Overview
Fixes a silent runtime bug from duplicate DialogManager methods and syncs tooling/docs to actual Python 3.14 support. Scope is appropriately narrow — no new features, only correctness and cleanup.
Bug Fix: Duplicate DialogManager Methods
This is a genuine fix. Python silently shadows earlier method definitions when two share the same name. The 3-arg show_transfer_stats was masked by the later 2-arg version, yet TransferManager called it with 3 positional args — a guaranteed TypeError inside a Tk.after() callback that was swallowed silently, causing the stats window to never appear after folder transfers. Removing the shadowed copy and dropping the extra arg at the call site is the right minimal fix.
Concern — stats formatting regression: The removed show_transfer_stats had richer output (total_files, transferred, skipped, bytes_saved fields). The surviving version renders a generic message dump. The PR correctly explains why (the with_dedup stubs only return a generic message dict so the structured fields would KeyError anyway). That reasoning is sound. But consider a TODO comment in the surviving method so the structured rendering is not forgotten once the dedup stubs land.
Concern — deduplicator: Removing the deduplicator arg from the caller is correct now, but the dialog will need it back for byte formatting once the dedup stubs are implemented. Worth a follow-up issue or TODO note.
Python 3.14 Tooling Sync
Adding py314 to Black target-version correctly matches requires-python = '>=3.13, <3.15'. One thing to verify: py314 support in Black was added in Black 25.x. Confirm the pinned version supports it, or users on older installs may get a config validation error.
mypy stays at python_version = '3.13' — correct call. Targeting the floor keeps type checking stable and conservative.
CLAUDE.md update is accurate.
Missing Test Coverage
No tests are added or modified. Given this fixes a silent runtime failure, regression tests would be valuable:
Verify show_disable_debugging_reminder() can be called without args without raising
A TransferManager integration test verifying the stats callback does not TypeError (even with a mocked dialog)
These would have caught the original bug and guard against reintroducing it.
Minor
The -> None return type annotations added to the surviving methods are correct and welcome.
The removed show_disable_debugging_reminder used 'Disable USB Debugging' as the dialog title; the surviving one uses 'Security Reminder'. Both are reasonable, but this title change is not mentioned in the PR description — worth a quick confirmation it is intentional.
Summary
Area
Status
Bug fix correctness
Correct and minimal
Tooling sync
Correct — verify Black version supports py314
Type annotations
Good
Documentation
Good
Test coverage
No regression tests added
Follow-up
TODO for dedup stats rendering; deduplicator removal
Overall a clean, well-scoped fix. Main asks: (1) add a TODO in show_transfer_stats so the structured stats rendering is not lost when the with_dedup stubs are implemented, and (2) add at least one regression test for the dialog call signature.
**Code Review — PR 48: Fix duplicate DialogManager methods and make tooling Python 3.14-aware**
**Overview**
Fixes a silent runtime bug from duplicate DialogManager methods and syncs tooling/docs to actual Python 3.14 support. Scope is appropriately narrow — no new features, only correctness and cleanup.
---
**Bug Fix: Duplicate DialogManager Methods**
This is a genuine fix. Python silently shadows earlier method definitions when two share the same name. The 3-arg show_transfer_stats was masked by the later 2-arg version, yet TransferManager called it with 3 positional args — a guaranteed TypeError inside a Tk.after() callback that was swallowed silently, causing the stats window to never appear after folder transfers. Removing the shadowed copy and dropping the extra arg at the call site is the right minimal fix.
Concern — stats formatting regression: The removed show_transfer_stats had richer output (total_files, transferred, skipped, bytes_saved fields). The surviving version renders a generic message dump. The PR correctly explains why (the with_dedup stubs only return a generic message dict so the structured fields would KeyError anyway). That reasoning is sound. But consider a TODO comment in the surviving method so the structured rendering is not forgotten once the dedup stubs land.
Concern — deduplicator: Removing the deduplicator arg from the caller is correct now, but the dialog will need it back for byte formatting once the dedup stubs are implemented. Worth a follow-up issue or TODO note.
---
**Python 3.14 Tooling Sync**
- Adding py314 to Black target-version correctly matches requires-python = '>=3.13, <3.15'. One thing to verify: py314 support in Black was added in Black 25.x. Confirm the pinned version supports it, or users on older installs may get a config validation error.
- mypy stays at python_version = '3.13' — correct call. Targeting the floor keeps type checking stable and conservative.
- CLAUDE.md update is accurate.
---
**Missing Test Coverage**
No tests are added or modified. Given this fixes a silent runtime failure, regression tests would be valuable:
- Verify show_transfer_stats accepts exactly (stats, operation) — 2 args — without raising
- Verify show_disable_debugging_reminder() can be called without args without raising
- A TransferManager integration test verifying the stats callback does not TypeError (even with a mocked dialog)
These would have caught the original bug and guard against reintroducing it.
---
**Minor**
- The -> None return type annotations added to the surviving methods are correct and welcome.
- The removed show_disable_debugging_reminder used 'Disable USB Debugging' as the dialog title; the surviving one uses 'Security Reminder'. Both are reasonable, but this title change is not mentioned in the PR description — worth a quick confirmation it is intentional.
---
**Summary**
| Area | Status |
|------|--------|
| Bug fix correctness | Correct and minimal |
| Tooling sync | Correct — verify Black version supports py314 |
| Type annotations | Good |
| Documentation | Good |
| Test coverage | No regression tests added |
| Follow-up | TODO for dedup stats rendering; deduplicator removal |
Overall a clean, well-scoped fix. Main asks: (1) add a TODO in show_transfer_stats so the structured stats rendering is not lost when the with_dedup stubs are implemented, and (2) add at least one regression test for the dialog call signature.
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Summary
Two fixes, both confined to existing-state cleanup (no new features).
1. Duplicate
DialogManagermethods → runtimeTypeErrorshow_transfer_statsandshow_disable_debugging_reminderwere each defined twice; the later definitions silently shadowed the earlier ones. The liveshow_transfer_stats(self, stats, operation)took only two args, butTransferManagerpassed a third (deduplicator), raisingTypeErrorinside the Tkafter()callback — so the transfer-stats window never appeared after folder transfers (it failed silently with a console traceback).The shadowed copies were also dead code: they indexed
stats["bytes_saved"]/["total_files"]/etc. — keys the*_with_dedupTODO stubs inadb_manager.pynever produce (they return{"message": ...}), so they'd haveKeyError'd on real data.deduplicatorargument at the caller-> NoneBehavior change: the transfer-stats window will now actually appear after folder transfers, rendering
Message: <…>(generic dump). The richer structured summary depends on implementing the*_with_dedupTODO and is out of scope here.2. Python 3.14 tooling/doc sync
The project already declares
requires-python = ">=3.13, <3.15"and CI tests['3.13','3.14'], but black/docs ignored 3.14.black target-version→['py313', 'py314']CLAUDE.md→ "Requires Python 3.13 or 3.14 (>=3.13, <3.15)"mypyintentionally kept at the 3.13 floor (oldest supported runtime)Verification
python -m src.main, no traceback)no-redeffor both methods gone🤖 Generated with Claude Code
Code review posted below - please see the review summary.
Code Review — PR 48: Fix duplicate DialogManager methods and make tooling Python 3.14-aware
Overview
Fixes a silent runtime bug from duplicate DialogManager methods and syncs tooling/docs to actual Python 3.14 support. Scope is appropriately narrow — no new features, only correctness and cleanup.
Bug Fix: Duplicate DialogManager Methods
This is a genuine fix. Python silently shadows earlier method definitions when two share the same name. The 3-arg show_transfer_stats was masked by the later 2-arg version, yet TransferManager called it with 3 positional args — a guaranteed TypeError inside a Tk.after() callback that was swallowed silently, causing the stats window to never appear after folder transfers. Removing the shadowed copy and dropping the extra arg at the call site is the right minimal fix.
Concern — stats formatting regression: The removed show_transfer_stats had richer output (total_files, transferred, skipped, bytes_saved fields). The surviving version renders a generic message dump. The PR correctly explains why (the with_dedup stubs only return a generic message dict so the structured fields would KeyError anyway). That reasoning is sound. But consider a TODO comment in the surviving method so the structured rendering is not forgotten once the dedup stubs land.
Concern — deduplicator: Removing the deduplicator arg from the caller is correct now, but the dialog will need it back for byte formatting once the dedup stubs are implemented. Worth a follow-up issue or TODO note.
Python 3.14 Tooling Sync
Missing Test Coverage
No tests are added or modified. Given this fixes a silent runtime failure, regression tests would be valuable:
These would have caught the original bug and guard against reintroducing it.
Minor
Summary
Overall a clean, well-scoped fix. Main asks: (1) add a TODO in show_transfer_stats so the structured stats rendering is not lost when the with_dedup stubs are implemented, and (2) add at least one regression test for the dialog call signature.