mysticalsoap/aqomui
Watch
1
0
Fork
You've already forked aqomui
 0
Code Issues 36 Pull requests 1 Releases 2 Packages Wiki Activity Actions

Module decomposition (index) #83

New issue
Open
opened 2026-08-20 10:48:59 -04:00 by mysticalsoap · 4 comments
mysticalsoap commented 2026-08-20 10:48:59 -04:00
Owner
Copy link

Index for module decomposition — one issue per seam, this issue for the doctrine and the map. AqomuiGui was 2,818 lines and 114 methods in one class when this was opened; the same tests apply to every module, not just the gui.

Doctrine (consolidated from the comment thread below, which carries the reasoning):

  • No big-bang rewrite — in this codebase, running finds what reading doesn't, and a rewrite discards exactly the knowledge encoded in working behavior. Incremental extractions on the #24 template: pick one responsibility, give it a typed home, move the consumers, delete the old paths, one live-verifiable PR each.
  • What earns extraction: a unit whose interface to the rest of the app is much narrower than its implementation. "It's consumed by the main gui file" disqualifies nothing — the main window is the composer.
  • Placement: a helper lives with its sole consumer; its own module only when shared across features. Modules a cli entry point imports stay Qt-free. Gui extractions land directly in aqomui/gui/, one module per feature/screen named for its main widget — not one file per QWidget subclass. widgets.py migrates piecemeal when work touches a class there anyway.
  • Tests mirror the package: tests/gui/test_.py.
  • Pacing: extractions ride along when work touches an area anyway — reference the seam issue from those PRs rather than scheduling pure-refactor batches.
  • Seams considered and rejected, with the failing test for each: see the 2026-08-23 comment below. Don't re-litigate them without new evidence.

Seams:

  • ServiceClient — PR #84 (unfinished half: the cli still runs its own hand-rolled proxy and should adopt ServiceClient)
  • Tray → gui/tray.py — PR #106
  • Autostart watcher → gui/autostart.py — PR #112
  • Network-bypass dialog, born directly in gui/netbypass.py per the amendment — PR #120
  • #126 gui-side JSON persistence → config.py
  • #125 server catalog model → aqomui/catalog.py (Qt-free)
  • #123 reconnect/autoconnect → a Qt-free module the cli can use
  • #127 Server tab → gui/servers.py
  • #128 Options tab → gui/options.py
  • #129 Provider tab → gui/provider.py
  • #130 Bypass tab → gui/bypass.py
  • #131 Profiles tab → gui/profiles.py (gated on #71)
  • Endpoint, after the seams above land: aqomui/service/ package (daemon + root mechanics), no interface/ wrapper — gui/ already encodes the Qt side, and the remaining top level is the shared Qt-free spine. One mechanical git-mv PR, plus an enforced no-gui-import rule for service/. Not earlier: today the import graph cuts across any such line (update.py reaches service, cli and widgets.py; launcher.py and bypass.py run on both sides), and modules still dissolving would move twice.

Related decomposition outside this class: #23 (shared structure for update.py's six providers, gated on parser test coverage). Feature decisions that reshape screens before extraction is worth doing: #82 (scripts), #86 (log tab), #70 (status+server collapse), #71 (profiles).

Index for module decomposition — one issue per seam, this issue for the doctrine and the map. AqomuiGui was 2,818 lines and 114 methods in one class when this was opened; the same tests apply to every module, not just the gui. **Doctrine** (consolidated from the comment thread below, which carries the reasoning): - No big-bang rewrite — in this codebase, running finds what reading doesn't, and a rewrite discards exactly the knowledge encoded in working behavior. Incremental extractions on the #24 template: pick one responsibility, give it a typed home, move the consumers, delete the old paths, one live-verifiable PR each. - **What earns extraction**: a unit whose interface to the rest of the app is much narrower than its implementation. "It's consumed by the main gui file" disqualifies nothing — the main window is the composer. - **Placement**: a helper lives with its sole consumer; its own module only when shared across features. Modules a cli entry point imports stay Qt-free. Gui extractions land directly in aqomui/gui/, one module per feature/screen named for its main widget — not one file per QWidget subclass. widgets.py migrates piecemeal when work touches a class there anyway. - Tests mirror the package: tests/gui/test_<module>.py. - **Pacing**: extractions ride along when work touches an area anyway — reference the seam issue from those PRs rather than scheduling pure-refactor batches. - Seams considered and **rejected**, with the failing test for each: see the 2026-08-23 comment below. Don't re-litigate them without new evidence. **Seams:** - [x] ServiceClient — PR #84 (unfinished half: the cli still runs its own hand-rolled proxy and should adopt ServiceClient) - [x] Tray → gui/tray.py — PR #106 - [x] Autostart watcher → gui/autostart.py — PR #112 - [x] Network-bypass dialog, born directly in gui/netbypass.py per the amendment — PR #120 - [ ] #126 gui-side JSON persistence → config.py - [ ] #125 server catalog model → aqomui/catalog.py (Qt-free) - [ ] #123 reconnect/autoconnect → a Qt-free module the cli can use - [ ] #127 Server tab → gui/servers.py - [ ] #128 Options tab → gui/options.py - [ ] #129 Provider tab → gui/provider.py - [ ] #130 Bypass tab → gui/bypass.py - [ ] #131 Profiles tab → gui/profiles.py (gated on #71) - [ ] Endpoint, after the seams above land: aqomui/service/ package (daemon + root mechanics), no interface/ wrapper — gui/ already encodes the Qt side, and the remaining top level is the shared Qt-free spine. One mechanical git-mv PR, plus an enforced no-gui-import rule for service/. Not earlier: today the import graph cuts across any such line (update.py reaches service, cli and widgets.py; launcher.py and bypass.py run on both sides), and modules still dissolving would move twice. Related decomposition outside this class: #23 (shared structure for update.py's six providers, gated on parser test coverage). Feature decisions that reshape screens before extraction is worth doing: #82 (scripts), #86 (log tab), #70 (status+server collapse), #71 (profiles).
mysticalsoap commented 2026-08-20 19:25:07 -04:00
Author
Owner
Copy link

Amendment (2026-08-20): the "gui/ subpackage only after the tab split" line had the sequencing backwards. It was aimed at not reshuffling existing files prematurely — but it would route every new extraction through widgets.py, moving each piece twice and growing a second monolith to dismantle later.

Revised rule: new extractions land directly in aqomui/gui/, one file per widget/class, starting immediately (package cost is an __init__.py and a setup.py packages entry — nothing existing moves). widgets.py migrates piecemeal under the same pacing rule: when work touches a class there anyway, it moves out. Tests follow the same split: tests/test_gui_<widget>.py per module rather than growing test_aqomui_gui.py.

First instance: gui/tray.py (TrayIcon wrapping QSystemTrayIcon + menu — signals show_window/quit_app/connect_server out, connected(name)/disconnected() in), riding along with the #69/#102/#103 tray work. Also the natural home for #85 and #93.

Amendment (2026-08-20): the "gui/ subpackage only after the tab split" line had the sequencing backwards. It was aimed at not reshuffling existing files prematurely — but it would route every new extraction through widgets.py, moving each piece twice and growing a second monolith to dismantle later. Revised rule: **new extractions land directly in `aqomui/gui/`, one file per widget/class, starting immediately** (package cost is an `__init__.py` and a setup.py `packages` entry — nothing existing moves). `widgets.py` migrates piecemeal under the same pacing rule: when work touches a class there anyway, it moves out. Tests follow the same split: `tests/test_gui_<widget>.py` per module rather than growing `test_aqomui_gui.py`. First instance: `gui/tray.py` (`TrayIcon` wrapping QSystemTrayIcon + menu — signals `show_window`/`quit_app`/`connect_server` out, `connected(name)`/`disconnected()` in), riding along with the #69/#102/#103 tray work. Also the natural home for #85 and #93.
mysticalsoap commented 2026-08-20 20:29:09 -04:00
Author
Owner
Copy link

Refinement to the amendment's "one file per widget/class", after checking the layout of mature PyQt projects (Anki's aqt/, Calibre's gui2/, Picard's ui/): the unit is one module per feature/screen, named for its main widget, with the helpers only it uses riding along in the same file — not one file per QWidget subclass. Two different questions, two different tests:

  • What earns extraction from AqomuiGui: a unit whose interface to the rest of the app is much narrower than its implementation. (TrayIcon: ~80 lines behind 5 signals + 2 state methods + usable; before #106, AqomuiGui poked its internals from five handlers.) "It's consumed by the main gui file" disqualifies nothing — the main window is the composer, that's true of every feature, and it's how the class reached 2,800 lines.
  • Where a class lives once extracted: with its only consumer when exactly one widget module uses it (a delegate, a one-off button — interface ≈ implementation); its own module only when shared across features. A future gui/servers.py holds the tab plus its item delegate and filter proxy, not three files.

Tests mirror the package: tests/gui/test_<module>.py, with an __init__.py so pytest namespaces the subdirectory (established in #106).

Refinement to the amendment's "one file per widget/class", after checking the layout of mature PyQt projects (Anki's `aqt/`, Calibre's `gui2/`, Picard's `ui/`): the unit is **one module per feature/screen, named for its main widget, with the helpers only it uses riding along in the same file** — not one file per QWidget subclass. Two different questions, two different tests: - **What earns extraction from AqomuiGui**: a unit whose interface to the rest of the app is much narrower than its implementation. (TrayIcon: ~80 lines behind 5 signals + 2 state methods + `usable`; before #106, AqomuiGui poked its internals from five handlers.) "It's consumed by the main gui file" disqualifies nothing — the main window is the composer, that's true of every feature, and it's how the class reached 2,800 lines. - **Where a class lives once extracted**: with its only consumer when exactly one widget module uses it (a delegate, a one-off button — interface ≈ implementation); its own module only when shared across features. A future `gui/servers.py` holds the tab *plus* its item delegate and filter proxy, not three files. Tests mirror the package: `tests/gui/test_<module>.py`, with an `__init__.py` so pytest namespaces the subdirectory (established in #106).
mysticalsoap commented 2026-08-20 23:50:15 -04:00
Author
Owner
Copy link

Sharpening one thing in the doctrine above: Anki/Calibre/Picard are cited as precedent, and that reads as if they're aspirational. They're existence proofs that the gui/ layout is conventional, not exemplars of where it ends up — Anki's aqt/main.py and Calibre's gui2/ui.py are exactly the ever-growing composer class we're digging out of, behind perfectly normal directory structures. The layout alone saves nothing. The load-bearing parts of the doctrine are the ones that stand without precedent: the extraction test (interface much narrower than implementation), sole-consumer placement, and keeping CLI-backing modules Qt-free (established with gui/autostart.py in PR #112: shim logic stays in launcher.py because the CLI imports it; only the Qt-facing sliver lives in gui/).

Sharpening one thing in the doctrine above: Anki/Calibre/Picard are cited as precedent, and that reads as if they're aspirational. They're existence proofs that the gui/ layout is conventional, not exemplars of where it ends up — Anki's aqt/main.py and Calibre's gui2/ui.py are exactly the ever-growing composer class we're digging out of, behind perfectly normal directory structures. The layout alone saves nothing. The load-bearing parts of the doctrine are the ones that stand without precedent: the extraction test (interface much narrower than implementation), sole-consumer placement, and keeping CLI-backing modules Qt-free (established with gui/autostart.py in PR #112: shim logic stays in launcher.py because the CLI imports it; only the Qt-facing sliver lives in gui/).
mysticalsoap commented 2026-08-23 01:45:11 -04:00
Author
Owner
Copy link

Survey of every module (2026-08-23), filed as #125–#131. Seams considered and rejected, each with the test it fails, so they don't get re-litigated without new evidence:

  • Connection lifecycle as a controller (dispatch_status, the twelve status handlers, kill/kill_bypass/disconnect_bypass, establish_connection, progress bars, the timeout watchdog): fails the interface test. Every handler coordinates tray + status widgets + progress bars + tab switching + notifications + service calls, so the extracted interface would be as wide as the implementation — this coordination is the composer's job. #123 already takes the policy sliver, and its own text keeps progress bars and status handling with the gui.
  • Service bootstrap/recovery ui (check_other_instance, initialize_service, service_unavailable/service_recovered, restart_aqomui, the version check): PR #84 already took the mechanism; what remains is dialog policy wired into ServiceClient's callbacks. Interface ≈ implementation.
  • notify/messageBox: two small helpers; interface ≈ implementation.
  • update_check (auto-update pacing): ~30 lines; by the #123 metric the cli could want it, but it fails the narrowness test on size. Revisit if the cli grows auto-update.
  • update.py per-provider importers: no module seam. The six share one pipeline — temp-dir lifecycle, cert copying, ip allow-listing, the finished payload — so per-provider modules would each keep the whole pipeline as their interface. Their shared-structure work is #23, deliberately gated on parser test coverage. (That update.py drags Qt into the cli via GuardedThread is real but not load-bearing: both consumers already run Qt main loops.)
  • widgets.py wholesale split: covered by the standing pacing rule; each tab issue names the widgets that ride along with it. ServerWidget stays shared — the server list and the bypass app/net lists both consume it.
  • Status screen → gui/status.py: StatusOn/OffWidget already live outside the composer; what remains in AqomuiGui is ~20 lines of composition. #70 will reshape this screen — the extraction rides with that work.
  • Log tab: a read-only text box and a level combo; implementation as small as its interface, and #86 questions its existence.
  • Service-side bypass orchestration (apply_bypass, cgroup_vpn, teardown_bypass/reconcile_bypass/sweep_bypass_units): orchestration glue over settings + tunnel_state + cgroup owner — an extracted class would need all of it passed in, no narrower. The mechanics already live in bypass.py.
  • tunnel.py internals (write_config, mgmt_events, the dns watchdog): methods with already-narrow interfaces inside their sole consumer TunnelThread; mgmt.py and dns_manager.py are the extractions that made sense here and already exist.
  • cli adoption of ServiceClient: real and outstanding, but an adoption, not an extraction — tracked on the index's ServiceClient line.

Found along the way, out of scope for this survey: the cli's two create_server_dict calls crash on the three-arg signature — filed as #132.

Survey of every module (2026-08-23), filed as #125–#131. Seams considered and **rejected**, each with the test it fails, so they don't get re-litigated without new evidence: - **Connection lifecycle as a controller** (`dispatch_status`, the twelve status handlers, `kill`/`kill_bypass`/`disconnect_bypass`, `establish_connection`, progress bars, the timeout watchdog): fails the interface test. Every handler coordinates tray + status widgets + progress bars + tab switching + notifications + service calls, so the extracted interface would be as wide as the implementation — this coordination *is* the composer's job. #123 already takes the policy sliver, and its own text keeps progress bars and status handling with the gui. - **Service bootstrap/recovery ui** (`check_other_instance`, `initialize_service`, `service_unavailable`/`service_recovered`, `restart_aqomui`, the version check): PR #84 already took the mechanism; what remains is dialog policy wired into ServiceClient's callbacks. Interface ≈ implementation. - **notify/messageBox**: two small helpers; interface ≈ implementation. - **update_check** (auto-update pacing): ~30 lines; by the #123 metric the cli could want it, but it fails the narrowness test on size. Revisit if the cli grows auto-update. - **update.py per-provider importers**: no module seam. The six share one pipeline — temp-dir lifecycle, cert copying, ip allow-listing, the finished payload — so per-provider modules would each keep the whole pipeline as their interface. Their shared-structure work is #23, deliberately gated on parser test coverage. (That update.py drags Qt into the cli via GuardedThread is real but not load-bearing: both consumers already run Qt main loops.) - **widgets.py wholesale split**: covered by the standing pacing rule; each tab issue names the widgets that ride along with it. `ServerWidget` stays shared — the server list and the bypass app/net lists both consume it. - **Status screen → gui/status.py**: StatusOn/OffWidget already live outside the composer; what remains in AqomuiGui is ~20 lines of composition. #70 will reshape this screen — the extraction rides with that work. - **Log tab**: a read-only text box and a level combo; implementation as small as its interface, and #86 questions its existence. - **Service-side bypass orchestration** (`apply_bypass`, `cgroup_vpn`, `teardown_bypass`/`reconcile_bypass`/`sweep_bypass_units`): orchestration glue over settings + tunnel_state + cgroup owner — an extracted class would need all of it passed in, no narrower. The mechanics already live in bypass.py. - **tunnel.py internals** (`write_config`, `mgmt_events`, the dns watchdog): methods with already-narrow interfaces inside their sole consumer TunnelThread; mgmt.py and dns_manager.py are the extractions that made sense here and already exist. - **cli adoption of ServiceClient**: real and outstanding, but an adoption, not an extraction — tracked on the index's ServiceClient line. Found along the way, out of scope for this survey: the cli's two `create_server_dict` calls crash on the three-arg signature — filed as #132.
mysticalsoap changed title from Decompose AqomuiGui to Module decomposition (index) 2026-08-23 01:45:32 -04:00
Sign in to join this conversation.
No Branch/Tag specified
Branches Tags
trunk
pin-proton-transport
change/network-observer
change/tray-state-icons
change/packaging-cleanup
fix/app-icons
change/icon
fix/import-file-modes
fix/service-input-validation
fix/dbus-policy
change/drop-self-update
fix/dco-device-detection
packaging/arch
v0.9.1
v0.9.0
Labels
Clear labels   
architecture
Structural change, not a local fix

  
bug
Defect in existing behaviour

  
cleanup
Tidying with no behaviour change

  
docs
Documentation and repo standards

  
packaging
distro packages and their build tooling

  
security
Vulnerability or hardening

  
testing
Test coverage or infrastructure

  
upstream
Needs action on someone else's project

  
ux
Interface behaviour and clarity

No labels
architecture
bug
cleanup
docs
packaging
security
testing
upstream
ux
Milestone
Clear milestone
No items
No milestone
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
mysticalsoap/aqomui#83
No description provided.