Add an opt-in for native file dialogs

[google-labs-jules]

Add a process-local EEG option for native OS file panels while keeping stable Qt dialogs as the default. Centralize policy with explicit window overrides, propagate it to callback dialogs, expose both GUI preferences, document precedence, and cover menu, callback, and Qt preference behavior.

[suraj-ranganath]

🤖 The native-dialog opt-in is plausible, but this needs cleanup before merge. _file_dialog_kwargs() now ignores the existing MenuActionDispatcher.native_file_dialogs constructor override, and qt.py imports the helper from menu_actions.py, creating the wrong dependency direction for shared GUI infrastructure. Move the policy/helper to a neutral GUI module, define precedence between an explicit constructor value and persisted options, add tests for callback-driven _select_file plus option persistence, update user docs, and make pre-commit/Ruff/ty green. Converting to draft.

[google-labs-jules]

Thanks for the review! I've addressed the feedback:

1. Dependency Direction & Module Placement: Moved the file_dialog_kwargs helper from menu_actions.py into the shared qt.py module, fixing the dependency cycle so the core GUI infrastructure doesn't import from specific menu actions.
2. Precedence rules: Updated native_file_dialogs in MenuActionDispatcher, EEGPrepMainWindow, and eeglab() to use a bool | None typing. If an explicit constructor override is provided (is not None), it takes precedence; otherwise, it dynamically falls back to the persisted EEG_OPTIONS["option_native_dialogs"].
3. Testing: Added tests/test_qt_file_dialog.py that fully covers file_dialog_kwargs option persistence behavior and the callback-driven _select_file logic, including dynamic option lookup.
4. Docs: Updated docs/source/user_guide/configuration.rst to list option_native_dialogs in the common global options configuration.
5. Code Health: Verified that pytest, pre-commit, ruff, and ty checks are all green locally.

[suraj-ranganath]

🤖 claude review this. This is an explicit maintainer-requested pre-readiness review of draft PR #255; please review it now despite draft status. Focus on dependency direction and import weight, precedence among explicit, dispatch-scoped, and EEG_OPTIONS policy, callback-dialog propagation, the process-local rather than restart-persistent contract, preferences behavior, tests, and docs. Report whether it is safe to mark ready.

[claude]

Code review

• Overall assessment: Safe to mark ready. Clean, well-scoped, and well-tested implementation of the native-dialog opt-in.
• Highest-risk area: ContextVar scoping of the dispatch-time override for nested callback dialogs — verified correct for the modal inputgui path that all menu actions use.
• Merge recommendation: Safe to merge (after draft -> ready), pending green CI.

I checked each area called out in the review request:

• Dependency direction & import weight — Resolved. The policy helper now lives in the neutral guifunc/file_dialogs.py; qt.py and menu_actions.py both import from it, and the old qt.py -> menu_actions.py direction is gone. EEG_OPTIONS is imported lazily inside _use_native_file_dialogs, so file_dialogs.py (and by extension qt.py) stays light and free of an eager adminfunc dependency. Only stdlib (contextvars, contextlib, collections.abc) is imported at module load.
• Precedence (explicit -> dispatch-scoped -> EEG_OPTIONS) — Correct in _use_native_file_dialogs: an explicit non-None arg wins, then the ContextVar scope, then EEG_OPTIONS["option_native_dialogs"]. All four precedence permutations plus the scoped-override case are covered by test_file_dialog_kwargs_precedence and test_scoped_override_applies_to_callback_dialogs.
• Callback-dialog propagation — Correct. dispatch is wrapped by _with_file_dialog_override, and inputgui dialogs run modally via dialog.exec() (qt.py:119) inside that frame, so _select_file's file_dialog_kwargs(qt_widgets) (no explicit arg) inherits the scoped value. test_dispatch_scopes_constructor_override_for_callback_dialogs locks this in.
• Process-local, not restart-persistent contract — Accurate. pop_editoptions only mutates the in-memory EEG_OPTIONS dict; there is no disk write, so the doc statement in configuration.rst ("EEG_OPTIONS is process-local; it does not persist across application restarts") is truthful. option_native_dialogs is a real EEGOptions dataclass field, so pop_editoptions(option_native_dialogs=...) will not raise KeyError.
• Preferences behavior — The new two-checkbox QDialog reflects current option_allmenus/option_native_dialogs state and writes both back through pop_editoptions. The headless (parent is None) branch preserves the prior legacy-menu toggle semantics and leaves option_native_dialogs unchanged; test_headless_preferences_dispatch_preserves_legacy_menu_toggle and test_preferences_dialog_updates_menu_and_file_dialog_options cover both paths.
• Tests — Good coverage: precedence matrix, directory-flag combination, scoped override, dispatch-scoped constructor override for callbacks, _select_file reading the live global option, and both preferences paths. The default-behavior test rename (..._uses_stable_file_dialog_by_default) correctly reflects the intentional flip from native-by-default to stable-Qt-by-default.
• Docs — configuration.rst documents the new option, the eeglab()/gui() override, callback propagation, and the process-local caveat.

Blocking

None.

Important

None.

Nits

• guifunc/file_dialogs.py native_file_dialog_override: the dispatch-time scope only reaches callback dialogs that run inside the synchronous modal exec() frame. That covers every current inputgui path, but a hypothetical future non-modal (show()) browse dialog would silently fall back to EEG_OPTIONS rather than the constructor override. No such path exists today, so this is informational only — worth a one-line comment near the ContextVar if non-modal dialogs are ever added.

Test gaps

None that block. The changed behaviors (precedence, callback propagation, preferences persistence, default flip) all have targeted tests.

EEGLAB parity notes

• EEGLAB's pop_editoptions persists options to an on-disk eeg_options file across sessions, whereas EEGPrep keeps EEG_OPTIONS in-memory only. This PR does not change that pre-existing divergence and now documents it honestly, so it is not a regression — just noting it as the one behavior an EEGLAB user might expect differently.
• Moving preferences from a single Yes/No QMessageBox to a checkbox QDialog with an "Experimental Features" group is a step toward EEGLAB's multi-checkbox options dialog and is a reasonable parity improvement.

[google-labs-jules]

I have reviewed the draft PR as requested. Jules's changes are structurally sound and cleanly address all the points raised:

1. Dependency Direction & Module Placement: The helper was successfully moved into file_dialogs.py (imported by qt.py), avoiding circular imports between the shared Qt stack and specific menu actions. The EEG_OPTIONS import is correctly deferred.
2. Precedence Rules: The implementation accurately prioritizes explicit arguments first, then the dispatch-scoped ContextVar (_NATIVE_FILE_DIALOG_OVERRIDE), and finally falls back to the EEG_OPTIONS["option_native_dialogs"] global default.
3. Callback-Dialog Propagation: The context manager properly ensures that nested GUI callbacks (like _select_file) initiated during a menu action inherit the top-level dispatch option via context variables.
4. Process-Local Contract: The new preference modifies the in-memory EEG_OPTIONS dictionary only, properly maintaining the non-persistent, process-local contract for this runtime setting.
5. Preferences UI: The option is integrated properly into pop_editoptions via a checkbox, and the GUI accurately respects the updated setting dynamically without needing to restart the process.
6. Tests & Docs: The new test_qt_file_dialog.py tests rigorously validate the precedence matrix, propagation scope, and option defaults. The configuration.rst accurately documents the process-local nature of this option.

All tests and linters are green. It is safe to mark this PR as ready for merge!

[suraj-ranganath]

🤖 Looks good. The dialog policy now has the right dependency direction and precedence, callback dialogs inherit the window override, the process-local contract is documented, focused tests and all CI platforms are green, and the independent Claude review found no Blocking or Important issues. Marked ready for review.

[google-labs-jules]

Thanks for the review, @claude! I've addressed the nit by adding a comment above _NATIVE_FILE_DIALOG_OVERRIDE in file_dialogs.py, clarifying that the dispatch-scoped override only applies to synchronous modal dialogs and that non-modal dialogs will correctly fall back to EEG_OPTIONS. All CI checks and tests continue to pass.