Decouple CTK from UI logic; lay groundwork for Qt6 by dxqb · Pull Request #1445 · Nerogar/OneTrainer

dxqb · 2026-05-10T12:28:15Z

All ~25 UI files in modules/ui/ mixed CustomTkinter widget construction with business logic, making it impossible to add a second toolkit without a full rewrite.

This PR applies a Controller / Base View / CTK View split to every window and tab:

Controller — owns all application logic with no UI toolkit dependency. For TrainUI, TrainUIController is what calls start_training(), stop_training(), handles progress callbacks, and opens tool windows.

Base View — toolkit-neutral widget layout. Receives a components namespace at construction and creates the toolkit-neutral layout using self.components.label(...), self.components.options(...) etc.

CTK View — inherits from both ctk.CTk and the base view. Creates CTK frames, and handles everything that cannot be toolkit-neutral.

No behavior changes intended. Method bodies moved verbatim where possible. Labels, tooltips, and option values untouched.

The pattern is similar to #1164 proposed by @HashakGik (but without the Model/Controller abstraction).
However, the git diffs are relative to the original files to enable human review and git tracability, hopefully enabling such a large refactor without introducing bugs.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

dxqb · 2026-05-10T12:53:02Z

All of this code is written by Claude Code, but with the goal to keep it human-reviewable and maintainable. I have reviewed lots of it.

@Koratahiu could you please review this refactor on the parts you have written originally? Optimizer parameters window, Muon Aux window, etc.
@O-J1 could you please do the same? Especially those parts that are less understood by myself, such as input validation. Are you happy with the split between validation.py and ctk_validation.py?
@efhosci could you please review this based on the video tools, statistics and other code you've written? I had to refactor video tools quite a bit to follow the pattern. I tested statistics but not video tools.

efhosci · 2026-05-10T18:28:45Z

Tested the video tool and it looks good, only noticed one minor issue where the "..." button is overlapping the end of the text input field for file selection. That probably just needs a change to the column width or padding. Otherwise all the download and extract functions seem to work the same as before. The statistics and image preview/augmentation stuff all seems good too. May go through the files in more detail later.

… VideoToolUI Removes abstract _create_browse_dir_button/_create_browse_file_button from BaseVideoToolUIView and uses the combined path_entry component instead, fixing widget alignment issues. Adds allow_video_files flag to path_entry. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

dxqb · 2026-05-11T18:22:40Z

Tested the video tool and it looks good, only noticed one minor issue where the "..." button is overlapping the end of the text input field for file selection. That probably just needs a change to the column width or padding. Otherwise all the download and extract functions seem to work the same as before. The statistics and image preview/augmentation stuff all seems good too. May go through the files in more detail later.

fixed by simplifying: there is a path_entry component already that can be used. you don't have to create an entry box and a button that opens file selection dialog. behaviour might have slightly changed, but it should only have improved: for example now there is input validation (a path that doesn't exist is marked red). please re-test anyway.

efhosci · 2026-05-12T04:09:17Z

Okay looks good now, thanks! Confirmed input validation works, invalid files are highlighted red. It looks like there are also a couple fields in the "Generate Captions" and "Generate Masks" windows that could be changed to use path_entry in a similar way, I think one of those was what I used as a reference for this originally.

@AbstractMethod

Base view concrete methods were accessing self.controller, self.ui_state, and toolkit-specific action methods that were only set by CTK subclass __init__ after the base __init__ call, with no enforcement in the base. - BaseTrainUIView: add controller/ui_state as constructor params; fix sync_cloud_secrets to use controller.train_config; add @AbstractMethod for export_training, generate_debug_package, open_profiling_tool - CtkTrainUIView: reorder __init__ to create deps before base init call; replace self.train_config with self.controller.train_config - BaseCloudTabView: add controller as constructor param - CtkCloudTabView: pass controller to base __init__, drop redundant assignment - BaseCaptionUIView: add ABC + @AbstractMethod for 6 action callbacks - BaseConceptTabView: remove concrete _update_filters() (accessed CTK vars); add ConceptConfig import; add concept: ConceptConfig param to BaseConceptWidgetView.__init__ - CtkConceptTabView: implement _update_filters(); pass concept to base init - BaseConceptWindowView: initialize bucket_ax/text_color/canvas to None - BaseTrainingTabView: replace callbacks dict with 6 @AbstractMethod declarations; restore_optimizer_config(variable: str) matches controller - CtkTrainingTabView: implement all 6 abstract methods directly Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

The ctypes DPI awareness call is toolkit-specific (fixes CTK transparency on Windows monitor changes). It already exists in CtkTrainUIView.py and has no place in the toolkit-agnostic controller. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

search_var/filter_var/show_disabled_var were stored in the base but never used there after _update_filters() was removed. Subclasses manage them. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Mirrors the change from upstream commit e928dda (Remove COFT Nerogar#1447), which removed COFT from LoraTab.py. The merge didn't carry it across the rename to CtkLoraTabView.py. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Brings in upstream changes (remove COFT Nerogar#1447, remove cautious_mask Nerogar#1451, fix aux optimizer defaults Nerogar#1444). Conflicts resolved by keeping the ctk_abstraction refactored versions (self.components, controller delegation). COFT removal applied manually to BaseLoraTabView. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…eep ctk_abstraction for UI conflicts - Apply LoKr and Scaled OFT additions to BaseLoraTabView/LoraTabController - Apply ETA session-tracking fix to TrainUIController - Apply local row counter fix to BaseTrainingTabView.__create_text_encoder_frame - Keep both supported_caption_extensions and json_path_modifier in path_util Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Was sitting on the same row at columns 3-4; now on its own row directly below the toggle for clearer visual grouping. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

…nto preview

O-J1

Reviewed validation and put an ask re: adding tests?

…erge Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

# Conflicts: # modules/ui/BaseConceptWindowView.py # modules/ui/CtkConceptWindowView.py

…nto preview

# Conflicts: # modules/ui/BaseConceptWindowView.py # modules/ui/BaseTimestepDistributionWindowView.py # modules/ui/BaseTrainingTabView.py # modules/ui/CtkConceptWindowView.py # modules/ui/CtkTimestepDistributionWindowView.py # modules/ui/CtkTrainingTabView.py

dxqb · 2026-06-26T18:36:20Z

Will not port dataset tools. It's a tool that isn't used very much, and can still be used using the Ctk UI for those who need it:

… loading) (Nerogar#1549)

previous_image/next_image called self.view.switch_image, but switch_image lives on the controller, not the view; arrow keys raised AttributeError. Also convert the _make_svd_frames docstring to a # comment. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

… split The Base/Ctk/Controller refactor deleted modules/ui/CaptionUI.py, ConvertModelUI.py and VideoToolUI.py, but the three entry scripts still imported them, crashing on launch with ModuleNotFoundError. Wire each script through the new controller + create_window(parent, view_cls) pattern (as TrainUIController already does internally). The new views are CTkToplevel, not CTk roots, so launch them under a hidden CTk root and wait_window on the toplevel to avoid a stray empty "tk" window and to exit cleanly on close. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

The ctk view refactor moved ConceptWindow.__download_dataset into the new ConceptWindowController.download_dataset from a base predating PR Nerogar#1524, which silently reverted that PR's fix: the method again called huggingface_hub.login(token=..., new_session=False) with no empty-token guard. new_session was removed from login() in huggingface-hub 1.16, so every call raised TypeError, swallowed by the surrounding except, so snapshot_download never ran and dataset download was fully broken. Re-apply Nerogar#1524 at the new location: only login when a token is configured, and drop new_session. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

The Base/Ctk refactor renamed UIState to BaseUIState (UIState.py) plus CtkUIState (CtkUIState.py) and updated validation.py, but missed the ctk sibling: ctk_validation.py still imported the now-nonexistent UIState under TYPE_CHECKING. Point it at CtkUIState, the concrete state actually passed to these ctk validators (mirroring validation.py -> BaseUIState). Invisible to ruff and the runtime (used in a string annotation, under TYPE_CHECKING, with future annotations), so only a type checker would flag it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

BaseConfigListView and BaseAdditionalEmbeddingsTabView imported customtkinter solely for a `-> ctk.CTkToplevel` return annotation on the abstract open_element_window, the lone toolkit reference in the Base layer (the sibling abstract methods return toolkit objects unannotated). Drop the annotation and the import so the Base layer is genuinely toolkit-free; the concrete Ctk subclasses keep their own CTkToplevel annotation. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…tk view split The ctk view split copied OptimizerParamsWindow into a controller from a base that predated Nerogar#1444, reverting `if not current_state:` back to `if current_state is None:`. Since muon_adam_config defaults to {} (not None), the empty initial state fell through to from_dict({}), so the Muon/aux-Adam window opened with bare default_values() instead of MUON_AUX_ADAM_DEFAULTS / ADAMW_ADV. Restore the upstream one-line guard. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…gc to close connect_window_closed bound <Destroy> unconditionally, so the callback fired once per descendant widget as the toplevel tore down. Guard on the event widget being the window itself so it fires exactly once. With that in place, open_sampling_tool defers torch_gc to window close via connect_window_closed instead of running it inline at open time, restoring the original 'free GPU memory after the sample window closes' behaviour. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

dxqb · 2026-06-29T21:51:17Z

code-reviewed as best as I can, but with this PR I'll have to rely on testing and AI partially. It wouldn't be possible otherwise

…bstraction Nerogar/merge received PR Nerogar#1564 as a squash-merge with tree identical to ctk_copy's tip. ctk_abstraction already contains all of ctk_copy's real commits, so -X ours is safe here: there is no content on Nerogar/merge beyond what's already in ctk_abstraction. # Conflicts: # modules/ui/BaseAdditionalEmbeddingsTabView.py # modules/ui/BaseCaptionUIView.py # modules/ui/BaseCloudTabView.py # modules/ui/BaseConceptTabView.py # modules/ui/BaseConceptWindowView.py # modules/ui/BaseConvertModelUIView.py # modules/ui/BaseGenerateCaptionsWindowView.py # modules/ui/BaseGenerateMasksWindowView.py # modules/ui/BaseLoraTabView.py # modules/ui/BaseModelTabView.py # modules/ui/BaseOffloadingWindowView.py # modules/ui/BaseProfilingWindowView.py # modules/ui/BaseSampleFrameView.py # modules/ui/BaseSampleParamsWindowView.py # modules/ui/BaseSampleWindowView.py # modules/ui/BaseSamplingTabView.py # modules/ui/BaseSchedulerParamsWindowView.py # modules/ui/BaseTimestepDistributionWindowView.py # modules/ui/BaseTopBarView.py # modules/ui/BaseTrainUIView.py # modules/ui/BaseTrainingTabView.py # modules/ui/BaseVideoToolUIView.py # modules/ui/CaptionUI.py # modules/ui/CaptionUIController.py # modules/ui/CtkGenerateCaptionsWindowView.py # modules/ui/CtkGenerateMasksWindowView.py # modules/ui/CtkSchedulerParamsWindowView.py # modules/ui/GenerateCaptionsWindow.py # modules/ui/GenerateMasksWindow.py # modules/ui/SampleWindow.py # modules/ui/SampleWindowController.py # modules/ui/SchedulerParamsWindow.py # modules/ui/VideoToolUI.py # modules/ui/VideoToolUIController.py

…erogar#1445) into pyside Nerogar/merge's tip is a squash-merge of ctk_abstraction, tree-identical to our ctk_abstraction/pyside_copy ancestors already in pyside's history. -X ours plus manual conflict resolution below keeps pyside's tree unchanged.

dxqb and others added 2 commits May 10, 2026 14:11

feat: split CTK UI into controller/base-view/ctk-view pattern

fe50e8d

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

refactor: abstract CTK views and migrate logic to controllers

a88f369

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

dxqb mentioned this pull request May 10, 2026

implement Qt6 views #1446

Merged

2 tasks

dxqb requested a review from O-J1 May 10, 2026 12:46

This comment was marked as resolved.

Sign in to view

dxqb and others added 11 commits May 15, 2026 11:40

refactor: remove unused __init__ from BaseConceptTabView

1bb5a56

search_var/filter_var/show_disabled_var were stored in the base but never used there after _update_filters() was removed. Subclasses manage them. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Merge branch 'upstream' into ctk_copy

9f42590

Merge branch 'upstream' into ctk_copy

3984c3f

Sync Ctk* copies with upstream changes merged into Base* files

c602458

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Move Rolling Backup Count under the Rolling Backup toggle

01cca79

Was sitting on the same row at columns 3-4; now on its own row directly below the toggle for clearer visual grouping. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Move Rolling Backup Count under the Rolling Backup toggle

647afc2

Was sitting on the same row at columns 3-4; now on its own row directly below the toggle for clearer visual grouping. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

dxqb added the preview merged in the preview branch label May 29, 2026

dxqb added a commit to TheForgotten69/OneTrainer that referenced this pull request Jun 3, 2026

Merge PR Nerogar#1445 into preview

5b32665

dxqb and others added 3 commits June 4, 2026 23:00

Merge branch 'upstream' into ctk_copy

408e7e2

Sync CtkConceptWindowView and ConceptWindowController with Base copy

a0bd3e4

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Merge ctk_copy: transformers 5.9, torch 2.12, compile thread-safety

3d90a9e

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

dxqb added a commit that referenced this pull request Jun 4, 2026

Merge PR #1445 (Decouple CTK from UI logic; lay groundwork for Qt6) i…

283cc60

…nto preview

O-J1 reviewed Jun 5, 2026

View reviewed changes

Comment thread modules/util/ui/validation.py

O-J1 reviewed Jun 5, 2026

View reviewed changes

dxqb and others added 2 commits June 13, 2026 11:01

Sync CtkConceptWindowView with BaseConceptWindowView after upstream m…

fdea894

…erge Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Merge branch 'ctk_copy' into ctk_abstraction

e70bada

# Conflicts: # modules/ui/BaseConceptWindowView.py # modules/ui/CtkConceptWindowView.py

dxqb added a commit that referenced this pull request Jun 14, 2026

Merge PR #1445 (Decouple CTK from UI logic; lay groundwork for Qt6) i…

47c1469

…nto preview

dxqb force-pushed the ctk_abstraction branch from f466854 to aee3609 Compare June 19, 2026 07:12

dxqb added a commit that referenced this pull request Jun 19, 2026

Merge PR #1445 (Decouple CTK from UI logic; lay groundwork for Qt6) i…

0e07592

…nto preview

dxqb added 3 commits June 19, 2026 20:27

Merge branch 'upstream' into ctk_copy

dd9bdbe

Sync Ctk* views with Base* after merging upstream

1f73b3a

dxqb and others added 10 commits June 26, 2026 20:50

Merge upstream: Disable LoRA rank check (false positive on Flux2 LoHA…

816c17a

… loading) (Nerogar#1549)

Merge ctk_copy: Disable LoRA rank check (false positive on Flux2 LoHA…

68b6a86

… loading) (Nerogar#1549)

Remove PLAN.md from .gitignore

57d53bf

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

dxqb added the merging last steps before merge label Jun 29, 2026

This was referenced Jul 1, 2026

ctk UI file split #1564

Merged

Qt UI file split #1565

Merged

dxqb deleted the branch Nerogar:merge July 1, 2026 04:16

dxqb closed this Jul 1, 2026

dxqb reopened this Jul 1, 2026

dxqb changed the base branch from ctk_copy to merge July 1, 2026 04:17

dxqb merged commit 75c4082 into Nerogar:merge Jul 1, 2026
1 check passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Decouple CTK from UI logic; lay groundwork for Qt6#1445

Decouple CTK from UI logic; lay groundwork for Qt6#1445
dxqb merged 34 commits into
Nerogar:mergefrom
dxqb:ctk_abstraction

dxqb commented May 10, 2026

Uh oh!

dxqb commented May 10, 2026 •

edited

Loading

Uh oh!

efhosci commented May 10, 2026

Uh oh!

This comment was marked as resolved.

dxqb commented May 11, 2026 •

edited

Loading

Uh oh!

efhosci commented May 12, 2026

Uh oh!

This comment was marked as resolved.

Uh oh!

O-J1 left a comment

Uh oh!

dxqb commented Jun 26, 2026 •

edited

Loading

Uh oh!

dxqb commented Jun 29, 2026 •

edited

Loading

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

dxqb commented May 10, 2026

Uh oh!

dxqb commented May 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

efhosci commented May 10, 2026

Uh oh!

This comment was marked as resolved.

dxqb commented May 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

efhosci commented May 12, 2026

Uh oh!

This comment was marked as resolved.

Uh oh!

O-J1 left a comment

Choose a reason for hiding this comment

Uh oh!

dxqb commented Jun 26, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dxqb commented Jun 29, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

dxqb commented May 10, 2026 •

edited

Loading

dxqb commented May 11, 2026 •

edited

Loading

dxqb commented Jun 26, 2026 •

edited

Loading

dxqb commented Jun 29, 2026 •

edited

Loading