diff --git a/docs/milestones/M05/M05_plan.md b/docs/milestones/M05/M05_plan.md index e698abf4e..a6713704d 100644 --- a/docs/milestones/M05/M05_plan.md +++ b/docs/milestones/M05/M05_plan.md @@ -1,62 +1,355 @@ # M05 Plan — Override Isolation / Temporary Opts Seam -**Milestone:** M05 -**Title:** Override isolation / temporary opts seam -**Branch:** `m05-override-isolation` -**Status:** Planned -**Depends on:** M04 (complete) +**Project:** Serena +**Phase:** Phase II — Runtime Seam Preparation +**Milestone:** M05 +**Title:** Override Isolation / Temporary Opts Seam +**Branch:** `m05-override-isolation` +**Posture:** Behavior-Preserving Refactor +**Target:** Introduce an isolated mechanism for temporary option overrides during generation. --- -## 1. Intent / Target +# 1. Intent / Target -Introduce the first architectural seam for Phase II: isolate override_settings application and restore from `process_images` into a reusable context manager or helper. This prepares for opts snapshot threading (M07–M08) and reduces direct mutation of global `shared.opts` during a run. +Introduce a **temporary options override seam** that prevents direct mutation of `shared.opts` during generation runs. -No runtime behavior changes. Override application and restore logic must remain identical. +Currently, `process_images()` temporarily mutates global options when applying `override_settings`, then restores them afterward. + +This milestone introduces a **context-managed override mechanism** that: + +* isolates temporary option changes +* preserves runtime behavior +* prepares the runtime pipeline for future **opts snapshot injection** + +This is the **first runtime seam milestone** in Phase II. --- -## 2. Scope Boundaries +# 2. Problem Being Solved + +The current implementation inside `modules/processing.py` roughly resembles: + +```python +for k, v in p.override_settings.items(): + opts.set(k, v) + +process_images_inner(p) + +restore_settings() +``` + +Issues: + +* mutates **global state** +* creates potential nondeterminism +* makes testing harder +* couples generation pipeline to `shared.opts` + +This milestone **does not change behavior**, but introduces a **structured override mechanism**. + +--- + +# 3. Scope Boundaries ### In scope -- Extract override apply/restore block in `process_images` into a context manager or helper -- Introduce `temporary_opts(override_settings)` or equivalent seam -- Preserve exact semantics: apply overrides before inner processing, restore in `finally` -- Add unit test for the seam (mock opts, verify apply/restore) +* Introduce a **temporary options context manager** +* Replace inline override logic in `process_images()` +* Ensure restoration logic is deterministic +* Preserve identical runtime semantics ### Explicitly out of scope -- Opts snapshot (immutable view) — M07 -- Passing opts into `process_images_inner` — M08 -- Changing override_settings semantics -- API or UI changes +* Changing how `opts` values are accessed elsewhere +* Introducing `opts_snapshot` (M07) +* Changing processing pipeline structure +* Any modification to API/UI behavior +* Performance changes --- -## 3. Current Behavior (Evidence) +# 4. Invariants (Must Not Change) -From `processing.py:823-857`: +The following runtime surfaces **must remain identical**: -- Override settings are applied to `shared.opts` via `opts.set(key, value)` before `process_images_inner` -- In `finally`, if `override_settings_restore_afterwards`, opts are restored -- This block is the target for extraction +| Surface | Verification | +| -------------------- | ----------------------------- | +| Image outputs | Smoke tests | +| API response schemas | API tests | +| CLI behavior | Smoke tests | +| Extension behavior | Extension loading smoke | +| Generation semantics | txt2img / img2img smoke tests | + +These invariants are part of the Serena invariant registry. --- -## 4. Implementation Approach +# 5. Verification Plan -1. Create helper or context manager (e.g. `modules/opts_override.py` or in `processing.py`) -2. Replace inline override block in `process_images` with call to the helper -3. Add minimal unit test that verifies apply/restore behavior -4. Ensure no behavior change; smoke and quality tests pass +### CI gates expected to remain green + +* Smoke tests +* Quality tests +* Coverage ≥ 40% +* verify_pinned_deps +* pip-audit (informational) + +### Evidence artifacts + +CI should still produce: + +``` +coverage.xml +ci_environment.txt +``` + +as introduced in M04. + +### Behavioral verification + +* Compare outputs from smoke generation tests +* Ensure override settings behave identically --- -## 5. Definition of Done +# 6. Implementation Steps -- [ ] Override apply/restore extracted to reusable seam -- [ ] `process_images` uses the seam; logic unchanged -- [ ] Unit test for seam -- [ ] Smoke and Quality CI green -- [ ] Milestone docs and ledger update +## Step 1 — Add temporary override context manager + +Create helper: + +``` +modules/runtime_utils.py +``` + +(or similar location appropriate to project structure) + +Add: + +```python +from contextlib import contextmanager +from modules import shared + + +@contextmanager +def temporary_opts(overrides: dict): + if not overrides: + yield + return + + original = {} + + try: + for key, value in overrides.items(): + if hasattr(shared.opts, key): + original[key] = getattr(shared.opts, key) + shared.opts.set(key, value) + + yield + + finally: + for key, value in original.items(): + shared.opts.set(key, value) +``` + +Purpose: + +* isolate override logic +* centralize restore semantics +* enable later replacement with snapshot model + +--- + +## Step 2 — Replace override block in `process_images()` + +Locate override logic inside: + +``` +modules/processing.py +``` + +Replace pattern similar to: + +```python +for k, v in p.override_settings.items(): + opts.set(k, v) + +process_images_inner(p) + +restore_settings() +``` + +with: + +```python +with temporary_opts(p.override_settings): + process_images_inner(p) +``` + +--- + +## Step 3 — Remove redundant restore logic + +Remove manual restore code now handled by context manager. + +Ensure behavior remains identical. + +--- + +## Step 4 — Minimal unit test + +Add small test under: + +``` +test/quality/test_opts_override.py +``` + +Example: + +```python +def test_temporary_opts_restores_value(): + from modules import shared + from modules.runtime_utils import temporary_opts + + original = shared.opts.some_option + + with temporary_opts({"some_option": "test_value"}): + assert shared.opts.some_option == "test_value" + + assert shared.opts.some_option == original +``` + +Purpose: + +* verify restoration behavior +* protect seam for future milestones + +--- + +## Step 5 — Ensure no behavior drift + +Run: + +``` +pytest +``` + +Verify: + +* generation tests unchanged +* API tests unchanged +* coverage still ≥ 40% + +--- + +# 7. Risk & Rollback Plan + +### Risk + +Low. + +Changes are isolated to override application. + +### Potential issue + +If extensions rely on exact ordering of override logic. + +### Rollback + +Revert the commit introducing: + +``` +temporary_opts +``` + +and restore original override block. + +Because the change is localized, rollback is trivial. + +--- + +# 8. Deliverables + +### Code + +New helper: + +``` +modules/runtime_utils.py +``` + +Modified: + +``` +modules/processing.py +``` + +New test: + +``` +test/quality/test_opts_override.py +``` + +--- + +### Documentation + +Update milestone artifacts: + +``` +docs/milestones/M05/M05_summary.md +docs/milestones/M05/M05_audit.md +``` + +Update ledger: + +``` +docs/serena.md +``` + +--- + +# 9. Acceptance Criteria + +M05 is complete when: + +* CI passes +* Coverage ≥ 40% +* Override logic replaced with context manager +* Runtime behavior unchanged +* Milestone documentation completed +* Audit score remains **5.0** + +--- + +# 10. Expected Architectural Impact + +Before: + +``` +process_images + └─ mutate shared.opts +``` + +After: + +``` +process_images + └─ temporary_opts + └─ process_images_inner +``` + +This creates the **first runtime seam** required for Phase II. + +--- + +# 11. Next Milestone + +``` +M06 — Prompt / Seed Preparation Extraction +``` + +Goal: + +Extract prompt + seed preparation logic from `process_images_inner()`. diff --git a/docs/milestones/M05/M05_toolcalls.md b/docs/milestones/M05/M05_toolcalls.md index 36a167248..04ac05769 100644 --- a/docs/milestones/M05/M05_toolcalls.md +++ b/docs/milestones/M05/M05_toolcalls.md @@ -7,4 +7,7 @@ | Timestamp | Tool | Purpose | Files/Target | Status | |-----------|------|---------|--------------|--------| -| (seeded) | — | M05 plan and toolcalls scaffold | docs/milestones/M05/ | done | +| 2026-03-09 | write | Create runtime_utils.py | modules/runtime_utils.py | done | +| 2026-03-09 | search_replace | Refactor process_images to use temporary_opts | modules/processing.py | done | +| 2026-03-09 | write | Add test_opts_override.py | test/quality/test_opts_override.py | done | +| 2026-03-09 | ruff | Lint runtime_utils, test_opts_override | modules/runtime_utils.py, test/quality/test_opts_override.py | pass | diff --git a/modules/processing.py b/modules/processing.py index 7535b56e1..3aa4c45c6 100644 --- a/modules/processing.py +++ b/modules/processing.py @@ -26,6 +26,7 @@ import modules.paths as paths import modules.face_restoration import modules.images as images import modules.styles +import modules.runtime_utils as runtime_utils import modules.sd_models as sd_models import modules.sd_vae as sd_vae from ldm.data.util import AddMiDaS @@ -820,42 +821,32 @@ def process_images(p: StableDiffusionProcessing) -> Processed: if p.scripts is not None: p.scripts.before_process(p) - stored_opts = {k: opts.data[k] if k in opts.data else opts.get_default(k) for k in p.override_settings.keys() if k in opts.data} + # if no checkpoint override or the override checkpoint can't be found, remove override entry and load opts checkpoint + # and if after running refiner, the refiner model is not unloaded - webui swaps back to main model here, if model over is present it will be reloaded afterwards + if sd_models.checkpoint_aliases.get(p.override_settings.get('sd_model_checkpoint')) is None: + p.override_settings.pop('sd_model_checkpoint', None) + sd_models.reload_model_weights() try: - # if no checkpoint override or the override checkpoint can't be found, remove override entry and load opts checkpoint - # and if after running refiner, the refiner model is not unloaded - webui swaps back to main model here, if model over is present it will be reloaded afterwards - if sd_models.checkpoint_aliases.get(p.override_settings.get('sd_model_checkpoint')) is None: - p.override_settings.pop('sd_model_checkpoint', None) - sd_models.reload_model_weights() + with runtime_utils.temporary_opts(p.override_settings, restore_afterwards=p.override_settings_restore_afterwards): + for k in p.override_settings: + if k == 'sd_model_checkpoint': + sd_models.reload_model_weights() + if k == 'sd_vae': + sd_vae.reload_vae_weights() - for k, v in p.override_settings.items(): - opts.set(k, v, is_api=True, run_callbacks=False) + sd_models.apply_token_merging(p.sd_model, p.get_token_merging_ratio()) - if k == 'sd_model_checkpoint': - sd_models.reload_model_weights() - - if k == 'sd_vae': - sd_vae.reload_vae_weights() - - sd_models.apply_token_merging(p.sd_model, p.get_token_merging_ratio()) - - # backwards compatibility, fix sampler and scheduler if invalid - sd_samplers.fix_p_invalid_sampler_and_scheduler(p) - - with profiling.Profiler(): - res = process_images_inner(p) + # backwards compatibility, fix sampler and scheduler if invalid + sd_samplers.fix_p_invalid_sampler_and_scheduler(p) + with profiling.Profiler(): + res = process_images_inner(p) finally: sd_models.apply_token_merging(p.sd_model, 0) - # restore opts to original state - if p.override_settings_restore_afterwards: - for k, v in stored_opts.items(): - setattr(opts, k, v) - - if k == 'sd_vae': - sd_vae.reload_vae_weights() + if p.override_settings_restore_afterwards and 'sd_vae' in p.override_settings: + sd_vae.reload_vae_weights() return res diff --git a/modules/runtime_utils.py b/modules/runtime_utils.py new file mode 100644 index 000000000..c8944eb1a --- /dev/null +++ b/modules/runtime_utils.py @@ -0,0 +1,41 @@ +"""Runtime utilities for the generation pipeline. + +M05: Temporary opts override seam — isolates option mutation during generation. +""" +from contextlib import contextmanager + +from modules import shared + + +@contextmanager +def temporary_opts(overrides: dict, restore_afterwards: bool = True): + """Context manager for temporary option overrides during generation. + + Applies overrides via opts.set(..., is_api=True, run_callbacks=False). + Restores original values on exit via setattr (no callbacks). + Only mutates keys present in opts.data. + + Args: + overrides: Dict of option key -> value to apply. + restore_afterwards: If True, restore original values on exit. + """ + opts = shared.opts + if not overrides: + yield + return + + original = { + k: opts.data[k] if k in opts.data else opts.get_default(k) + for k in overrides.keys() + if k in opts.data + } + + try: + for k, v in overrides.items(): + if k in opts.data: + opts.set(k, v, is_api=True, run_callbacks=False) + yield + finally: + if restore_afterwards: + for k, v in original.items(): + setattr(opts, k, v) diff --git a/test/quality/test_opts_override.py b/test/quality/test_opts_override.py new file mode 100644 index 000000000..df1e984c0 --- /dev/null +++ b/test/quality/test_opts_override.py @@ -0,0 +1,34 @@ +"""Unit tests for temporary_opts context manager (M05 override isolation seam).""" +from modules import shared +from modules.runtime_utils import temporary_opts + + +def test_temporary_opts_restores_value(initialize): + """temporary_opts restores samples_save to original value on exit.""" + original = shared.opts.samples_save + + with temporary_opts({"samples_save": False}): + assert shared.opts.samples_save is False + + assert shared.opts.samples_save == original + + +def test_temporary_opts_restore_afterwards_false(initialize): + """When restore_afterwards=False, value is not restored.""" + original = shared.opts.samples_save + try: + with temporary_opts({"samples_save": False}, restore_afterwards=False): + assert shared.opts.samples_save is False + assert shared.opts.samples_save is False + finally: + shared.opts.samples_save = original + + +def test_temporary_opts_empty_overrides(initialize): + """Empty overrides yields without mutation.""" + original = shared.opts.samples_save + + with temporary_opts({}): + pass + + assert shared.opts.samples_save == original