refactor(desktop): single-source result-kind CSV spec + reviewed Batch 10 plan (#80)

* refactor(desktop): single-source result-kind CSV headers + filename (dedup)

The (kind -> CSV base headers + export filename) mapping was hardcoded 2-4x per
kind — once in each _show_*_results mixin (first render) and again in
_refresh_display_format (reformat). Changing a header meant editing both or the
reformat path would silently emit stale headers.

Introduce app_desktop/result_csv_spec.py (a leaf module, no app_desktop imports,
so window.py and the mixins it composes can both import it without a cycle) with
result_csv_headers(kind)/result_csv_filename(kind). Route the extrapolation and
statistics first-render paths and the extrapolation/statistics/fit_single
reformat branches through it. The statistics dynamic *_unit column extension and
error/snapshot dynamic-header paths are unchanged.

This is the narrow, low-risk dedup — NOT the larger _refresh_display_format
polymorphic refactor, which was consciously declined (result kinds are not added
frequently and the 11 branches diverge too much for a clean abstraction).

New test asserts the spec values, that headers are a fresh mutable copy, and
(via getsource) that neither path re-hardcodes the literals — RED when a literal
is re-inlined. ruff clean; 122 desktop tests pass.

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

* docs: Batch 10 window-decomposition plan, revised after Codex adversarial review

Staged plan (no code) for decomposing the 3197-line ExtrapolationWindow god-class
via the proven window_fitting_mixin.py shim pattern. Revised per external
adversarial review (Codex/gpt-5.5) which flagged real gaps:
- Stage 2 (extrapolation mixin) NO-GO as written: it's a cross-family run
  controller (run_calculation dispatches stats/fitting/root; owns unit
  collectors), not extrapolation-only. Re-scope + park.
- Stage 1 CONDITIONAL: _on_stats_mode_change lives in window.py, not the stats
  mixin — keep it out of the split.
- Stage 0 expanded: MRO/provider-order snapshots, shim-override checks (the
  fitting shim is NOT empty — it overrides _on_fit_finished), no-__init__ checks,
  and enumerate direct-import sites of internal helpers (tests import them).
- Stage 4 (Protocol facade) NO-GO for Batch 10: 182 owner.* names; do as narrow
  per-view typing spikes later.
Revised order: 0 -> 3 (latex, cleanest) -> 1 (stats, small PRs) -> 2 (re-scoped).

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

* docs: fold Gemini (Antigravity) findings into Batch 10 plan (two-model review)

Second external review (Antigravity/Gemini) verdict REJECT — agreed with all
Codex findings and added three verified hazards:
- Qt MRO severance: QMainWindow is the leftmost base (window.py:467); PySide6 C++
  wrappers don't cooperatively super(), so a split mixin overriding a Qt event
  handler (closeEvent/resizeEvent/__init__) is silently ignored. Stage 0 must
  forbid Qt-event overrides in mixins.
- Precedence reversal: splitting a monolith bottom->top and composing
  Shim(A_top, B, C_bottom) reverses shadowing (A shadows C under left-to-right
  MRO). Stage 0 must prohibit duplicate method names across siblings.
- Stage 3: _TectonicInstallWorker (:55) and _LatexCompileWorker (:135) are
  defined inside window_latex_pdf_mixin.py while all other workers live in
  workers_qt.py — extract them for consistency.
All re-verified against the code. Plan remains a plan (no code executed).

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

---------

Co-authored-by: fanghao <fanghaoaa@gmail.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

Author: 3 people

app_desktop/result_csv_spec.py

@@ -0,0 +1,40 @@
+"""Single source of truth for each result kind's CSV base headers + export
+filename.
+
+Both the first-render path (the ``_show_*_results`` methods in the
+``window_*_mixin`` files) and the reformat path
+(``ExtrapolationWindow._refresh_display_format``) read from here, so a
+header/filename change is made once and can't drift between the two paths
+(audit R3 — the same header literals + filenames were duplicated 2-4x).
+
+Kinds whose CSV headers are computed dynamically are intentionally absent:
+- ``error`` appends ``output_unit`` when a unit is present;
+- ``statistics`` may append ``value_unit``/``uncertainty_unit`` columns;
+- snapshot-based kinds (statistics_matrix/bootstrap/hypothesis/time_series/
+  grouped, fitting_comparison) derive their headers from the rendered snapshot.
+
+This is a leaf module (no ``app_desktop`` imports) so both ``window.py`` and the
+mixins it composes can import it without a circular dependency.
+"""
+
+from __future__ import annotations
+
+_RESULT_CSV_SPEC: dict[str, tuple[tuple[str, ...], str]] = {
+    "extrapolation": (("index", "value", "uncertainty", "latex"), "extrapolation_results.csv"),
+    "statistics": (("batch", "metric", "value", "uncertainty"), "statistics_results.csv"),
+    "fit_single": (
+        ("batch", "section", "name", "value", "uncertainty", "stat_error", "sys_error", "note"),
+        "fitting_results.csv",
+    ),
+}
+
+
+def result_csv_headers(kind: str) -> list[str]:
+    """Base CSV headers for a result kind (fresh mutable copy, so callers may
+    extend it with dynamic unit columns)."""
+    return list(_RESULT_CSV_SPEC[kind][0])
+
+
+def result_csv_filename(kind: str) -> str:
+    """Suggested export filename for a result kind."""
+    return _RESULT_CSV_SPEC[kind][1]

app_desktop/window.py

@@ -169,6 +169,7 @@
 
 from .about_dialog import show_about_dialog
 from .docs_dialog import DocsDialog
+from .result_csv_spec import result_csv_filename, result_csv_headers
 from .resources import (
     _apply_system_theme,
     _compute_default_pdf_dpi,
@@ -2912,7 +2913,7 @@ def _refresh_display_format(self):
                 text, csv_rows = self._format_extrapolation_display(**payload)
                 self._set_result_text(text)
                 if csv_rows:
-                    self._set_csv_data(csv_rows, ["index", "value", "uncertainty", "latex"], suggestion="extrapolation_results.csv")
+                    self._set_csv_data(csv_rows, result_csv_headers("extrapolation"), suggestion=result_csv_filename("extrapolation"))
                 else:
                     self._reset_csv_data()
             elif kind == "error":
@@ -2932,14 +2933,14 @@ def _refresh_display_format(self):
                 text, csv_rows = self._format_statistics_display(**payload)
                 self._set_result_text(text)
                 if csv_rows:
-                    self._set_csv_data(csv_rows, ["batch", "metric", "value", "uncertainty"], suggestion="statistics_results.csv")
+                    self._set_csv_data(csv_rows, result_csv_headers("statistics"), suggestion=result_csv_filename("statistics"))
                 else:
                     self._reset_csv_data()
             elif kind == "statistics_batches":
                 text, csv_rows = self._format_statistics_batches_display(**payload)
                 self._set_result_text(text)
                 if csv_rows:
-                    self._set_csv_data(csv_rows, ["batch", "metric", "value", "uncertainty"], suggestion="statistics_results.csv")
+                    self._set_csv_data(csv_rows, result_csv_headers("statistics"), suggestion=result_csv_filename("statistics"))
                 else:
                     self._reset_csv_data()
             elif kind in {
@@ -2966,8 +2967,8 @@ def _refresh_display_format(self):
                 if csv_rows:
                     self._set_csv_data(
                         csv_rows,
-                        ["batch", "section", "name", "value", "uncertainty", "stat_error", "sys_error", "note"],
-                        suggestion="fitting_results.csv",
+                        result_csv_headers("fit_single"),
+                        suggestion=result_csv_filename("fit_single"),
                     )
                 else:
                     self._reset_csv_data()

app_desktop/window_extrapolation_mixin.py

@@ -23,6 +23,7 @@
     CalcJob,
 )
 from .workers_qt import CalcWorker, RootSolvingWorker
+from .result_csv_spec import result_csv_filename, result_csv_headers
 
 _DIRECT_STATISTICS_WORKFLOWS = {
     "bootstrap_confidence_intervals",
@@ -817,7 +818,7 @@ def _show_extrapolation_results(
                         figure_paths.append(img_path)
         self._set_result_text(text, final_result=True)
         if csv_rows:
-            self._set_csv_data(csv_rows, ["index", "value", "uncertainty", "latex"], suggestion="extrapolation_results.csv")
+            self._set_csv_data(csv_rows, result_csv_headers("extrapolation"), suggestion=result_csv_filename("extrapolation"))
         else:
             self._reset_csv_data()
         if render_plots and figure_paths:

app_desktop/window_statistics_mixin.py

@@ -43,6 +43,7 @@
 from datalab_latex.latex_tables_statistics_matrix import generate_statistics_matrix_latex
 
 from .parallel_preferences import current_parallel_config_from_widgets
+from .result_csv_spec import result_csv_filename, result_csv_headers
 from .workers_core import _mp_precision_guard, _safe_read_text
 
 
@@ -1643,10 +1644,10 @@ def _display_statistics_result(
         self._append_statistics_warning_logs(result)
         self._set_result_text(text, final_result=True)
         if csv_rows:
-            headers = ["batch", "metric", "value", "uncertainty"]
+            headers = result_csv_headers("statistics")
             if any("value_unit" in row or "uncertainty_unit" in row for row in csv_rows):
                 headers.extend(["value_unit", "uncertainty_unit"])
-            self._set_csv_data(csv_rows, headers, suggestion="statistics_results.csv")
+            self._set_csv_data(csv_rows, headers, suggestion=result_csv_filename("statistics"))
         else:
             self._reset_csv_data()
         remembered: dict[str, object] = {"result": result, "value_col": value_col, "n": n}

docs/BATCH10_WINDOW_DECOMPOSITION_PLAN.md

@@ -0,0 +1,174 @@
+# Batch 10 — `ExtrapolationWindow` decomposition (staged plan, no code yet)
+
+> R3-soft KEEP `[_idx 17]`; CODE_REVIEW_2026 §2.5 (high). **This document is a
+> plan only.** Each stage is a separate, independently-mergeable PR gated by the
+> project review workflow (cluster tests + 3-model + CodeRabbit + full-suite).
+>
+> **Revised after two-model external adversarial review (Codex / gpt-5.5 +
+> Antigravity / Gemini), each finding re-verified against the code.** Gemini's
+> verdict was **REJECT** (not safe to execute as originally written); it agreed
+> with every Codex finding and added the Qt-MRO-severance, precedence-reversal,
+> and Stage-3 worker-placement hazards below. Net: the shim/MRO approach is
+> viable, but "behavior-identical pure move" is only credible with the expanded
+> Stage-0 characterization first — original staging was ROI-first, this revision
+> is safety-first.
+>
+> ### Critical MRO hazards (Gemini, verified) — must be encoded in Stage 0
+> - **Qt base severs the MRO chain.** `ExtrapolationWindow(QMainWindow, …Mixins)`
+>   has `QMainWindow` **leftmost** (window.py:467). PySide6 C++ wrappers do NOT
+>   cooperatively call `super()`, so any Qt lifecycle override (`closeEvent`,
+>   `resizeEvent`, `__init__`) in a split mixin is **silently ignored**. → The
+>   plan MUST forbid split mixins from overriding Qt event handlers (or audit for
+>   dormant ones being ignored today).
+> - **Splitting reverses definition precedence.** In the monolith a method at the
+>   *bottom* shadows a duplicate at the *top*. Split sequentially into A(top) /
+>   B / C(bottom) and composed `class Shim(A, B, C)`, left-to-right MRO makes
+>   **A shadow C** — the opposite of the original. → Stage 0 must **strictly
+>   prohibit duplicate method names across sibling mixins** (a test that fails on
+>   any cross-sibling name collision).
+
+## Current state (measured)
+
+- `app_desktop/window.py` — **3197 lines**, `ExtrapolationWindow` with **~150
+  methods**, composed of **7 mixins** (MRO order):
+  `WindowLatexPdfMixin, WindowI18nMixin, WindowImagesMixin,
+  WindowStatisticsMixin, WindowDataMixin, WindowFittingMixin,
+  WindowExtrapolationMixin`.
+- Mixin sizes (the god-files):
+  - `window_statistics_mixin.py` — **1921** ← biggest
+  - `window_extrapolation_mixin.py` — **1128**
+  - `window_latex_pdf_mixin.py` — 969
+  - fitting already split into 4 (`formatters` 561 / `residuals` 614 /
+    `models` 673) behind a 67-line shim — **the proven pattern**.
+  - `window_data_mixin.py` 646, `window_i18n_mixin.py` 423,
+    `window_images_mixin.py` 388.
+- `app_desktop/views/` (extrapolation/fitting/statistics/error/root_solving.py,
+  ~3524 lines) already extracts **widget construction** via
+  `build_*_mode_view(owner)` functions. The mixins hold **behavior**
+  (run/format/handlers). The separation is: `views/` = build UI, mixins = drive
+  it.
+
+## Guiding constraints (why this is staged, not one refactor)
+
+1. **Public API stable.** `window.py` must keep inheriting the same mixin names;
+   external imports (`from app_desktop.window_* import Window*Mixin`) keep
+   working. Follow the `WindowFittingMixin` shim: an empty subclass that composes
+   split files and re-exports the original name. Zero caller changes per stage.
+2. **MRO discipline.** Method-resolution order is load-bearing (some mixins
+   override same-named methods). Any split must preserve left-to-right MRO;
+   pin it explicitly and document the order (as the fitting shim does).
+3. **Behavior-identical.** No logic changes inside a split — pure move. Verify
+   with the existing per-mode desktop UI tests + full suite each stage.
+4. **Surgical, reversible.** One mixin per PR. If a split reveals a real coupling
+   bug, fix it in its own commit, not silently.
+5. **Test coverage first.** Before splitting a mixin, confirm the per-mode UI
+   test file exercises its public methods (statistics/extrapolation/fitting all
+   have `test_desktop_*_ui.py`). Add characterization tests for any method with
+   no coverage BEFORE moving it.
+
+## Staged sequence (safest-first, one PR each)
+
+### Stage 0 — Guardrails (prep) — **GO, but expanded per review**
+- Characterization tests: for each mixin to split, list public methods + confirm
+  coverage; fill gaps with thin output-pinning tests. No production change.
+- **(review) MRO / provider-order snapshot:** capture `ExtrapolationWindow.__mro__`
+  and each split's base order in a test so a re-order is caught.
+- **(review) Shim overrides are NOT always empty:** `window_fitting_mixin.py:48`
+  overrides `_on_fit_finished` and calls `super()` before adding fallback-history
+  UI. Stage 0 must add checks for (a) duplicate method names across siblings,
+  (b) intentional shim-level overrides, (c) **no `__init__` in split mixins**
+  (construction order is load-bearing) — and a construction test proving signal
+  slots exist before `build_*_mode_view()` runs.
+- **(review) Import-stability is broader than `Window*Mixin`:** tests import
+  internal helpers directly, e.g. `_statistics_raw_table_preserving_cells` from
+  `window_statistics_mixin.py` (`test_desktop_statistics_ui.py:705`), and the
+  release matrix names old FQNs (`test_release_test_matrix.py:70`). Any split
+  must **re-export moved internals** from the original module OR update that
+  evidence deliberately — Stage 0 enumerates these direct-import sites.
+- Freeze the file-size ratchet baselines for the new split files.
+
+### Stage 1 — Split `window_statistics_mixin.py` (1921 → ~4 files) — biggest win
+Mirror the fitting shim. Suggested responsibility split (validate against the
+actual method groupings before coding):
+- `window_statistics_formatters_mixin.py` — result-text / CSV-row / snapshot
+  rendering helpers (pure-ish).
+- `window_statistics_modes_mixin.py` — per-mode execution + dispatch
+  (`_run_statistics_mode` and the standard/bootstrap/hypothesis/time-series/
+  matrix/grouped paths).
+- `window_statistics_results_mixin.py` — worker-result handlers + plot wiring.
+- `window_statistics_mixin.py` — 67-line shim composing the above, MRO-pinned,
+  re-exporting `WindowStatisticsMixin`. Public import unchanged.
+- **Verdict: CONDITIONAL-GO (review).** **`_on_stats_mode_change` lives in
+  `window.py`, NOT in the statistics mixin** (its visibility logic touches
+  `views/statistics.py:379` and is called from `workspace_controller.py:981`).
+  Do NOT pull it into Stage 1 — either freeze it explicitly in `window.py` or
+  make a separate stats-visibility stage. The snapshot/semantic-output path is
+  the other tricky seam. Split stats into smaller PRs if the first is too large.
+
+### Stage 2 — `window_extrapolation_mixin.py` (1128) — **NO-GO as written; re-scope (review)**
+**This mixin is misnamed for splitting purposes: it is a cross-family run
+controller, not extrapolation-only.** Its `run_calculation()` dispatches direct
+statistics + fitting paths (`window_extrapolation_mixin.py:389`),
+`_on_calc_finished()` handles statistics results (`:582`), and it owns the
+root/error/statistics/fitting **unit collectors** (`:1035`). A naive
+"extrapolation run" split would break stats/fitting/root/unit-propagation.
+Re-scope as a **cross-family run-controller split** with full mode coverage:
+first extract the pure formatters and the unit-collectors (self-contained), and
+only then consider separating the dispatch — treating it as its own multi-PR
+track, not a low-medium extrapolation task. F10's mode-switch/result-clear timing
+must move verbatim. **Park until Stage 1 + 3 are done and characterized.**
+
+### Stage 3 — Split `window_latex_pdf_mixin.py` (969 → ~2 files) — **GO (cleanest)**
+- Separate compile-worker orchestration from PDF-preview/zoom UI.
+- **(review, Gemini) Also extract the two workers.** `_TectonicInstallWorker`
+  (`window_latex_pdf_mixin.py:55`) and `_LatexCompileWorker` (`:135`) are `QThread`
+  subclasses defined *inside* this mixin, whereas all other workers
+  (`CalcWorker`, `FitWorker`, `RootSolvingWorker`, …) live in `workers_qt.py`.
+  Amend Stage 3 to move these two to `workers_qt.py` for architectural
+  consistency (do it as part of, or just before, the split).
+- **Risk: low.** Self-contained; good coverage in `test_update_*` /
+  `test_latex_compile_*`.
+
+### Stage 4 — Typed window facade `Protocol` — **NO-GO for Batch 10 (review)**
+A `WindowFacade` Protocol would have to cover **182 distinct `owner.*` names**
+across `app_desktop/views/*.py`, and the project already records broad desktop
+statistics mypy as blocked by dynamic-mixin-attr debt (`task_plan.md:277`). One
+giant Protocol is noisy and brittle. **Do not bundle into Batch 10.** If pursued
+later, do it as narrow **per-view / per-helper Protocols**, as its own typing
+spike — not a window-wide facade.
+
+### Explicitly NOT in scope
+- No "delegating controllers" rewrite / no MVC re-architecture. The mixin+shim
+  composition is the target end-state — it already fits every file in one editor
+  view and keeps the public API. A full controller rewrite is high-risk with no
+  proven payoff and is **not recommended**.
+
+## Effort / risk summary (post-review verdicts)
+
+| Stage | File | Effort | Risk | Verdict |
+|---|---|---|---|---|
+| 0 | (tests + MRO/import characterization) | S-M | none | **GO — expanded** |
+| 1 | statistics_mixin | M | medium | **CONDITIONAL-GO** (keep `_on_stats_mode_change` out; split into smaller PRs) |
+| 3 | latex_pdf_mixin | S-M | low | **GO — cleanest** |
+| 2 | extrapolation_mixin (run-controller) | L | med-high | **NO-GO as written** — re-scope as cross-family run-controller, park |
+| 4 | Protocol facade | L | — | **NO-GO for Batch 10** — separate narrow typing spike |
+
+**Revised recommendation (safety-first):**
+1. **Stage 0** — build the characterization/MRO/import-stability guardrails first
+   (this is what makes every later "pure move" provable). Do this next if Batch 10
+   proceeds.
+2. **Stage 3** (latex_pdf) — the cleanest, lowest-risk split; good second PR to
+   validate the shim mechanics on real code.
+3. **Stage 1** (statistics) — biggest win, but only after 0+3, and only with
+   `_on_stats_mode_change` frozen in `window.py`; split into small PRs.
+4. **Stage 2** (the run-controller) — re-scope and revisit LAST, or not at all.
+5. **Stage 4** — out of scope.
+
+Do NOT attempt a monolithic "decompose the whole window" PR. Order changed from
+1→2→3 to **0 → 3 → 1 → (2 re-scoped)** on review: do the provably-safe splits
+first, defer the cross-family run-controller.
+
+## Open question for maintainer
+- Proceed with Stages 0–3 now, or park Batch 10 (the god-class works, is tested,
+  and the value is maintainability-only)? The per-stage payoff is real but purely
+  structural — no user-facing change and no bug fixed.

tests/test_result_csv_spec.py

@@ -0,0 +1,60 @@
+"""The result-CSV spec is the single source of truth for each result kind's
+base CSV headers + export filename, shared by the first-render path (_show_*
+mixins) and the reformat path (_refresh_display_format) so they cannot drift
+(audit R3 CSV-spec dedup)."""
+
+from __future__ import annotations
+
+import inspect
+import linecache
+
+from app_desktop.result_csv_spec import (
+    _RESULT_CSV_SPEC,
+    result_csv_filename,
+    result_csv_headers,
+)
+
+
+def test_result_csv_headers_returns_fresh_mutable_copy():
+    a = result_csv_headers("extrapolation")
+    a.append("EXTRA")
+    b = result_csv_headers("extrapolation")
+    assert b == ["index", "value", "uncertainty", "latex"], "spec must not be mutated by callers"
+    assert "EXTRA" not in b
+
+
+def test_result_csv_spec_values():
+    assert result_csv_headers("extrapolation") == ["index", "value", "uncertainty", "latex"]
+    assert result_csv_filename("extrapolation") == "extrapolation_results.csv"
+    assert result_csv_headers("statistics") == ["batch", "metric", "value", "uncertainty"]
+    assert result_csv_filename("statistics") == "statistics_results.csv"
+    assert result_csv_headers("fit_single")[:4] == ["batch", "section", "name", "value"]
+    assert result_csv_filename("fit_single") == "fitting_results.csv"
+
+
+def test_reformat_and_first_render_read_the_same_spec():
+    """Both the reformat path (window._refresh_display_format) and the first-render
+    paths (window_*_mixin _show_* methods) must source headers/filename from the
+    spec — no hardcoded '..._results.csv' string literals should remain in those
+    methods, or the two paths can silently diverge again."""
+    from app_desktop import window as window_mod
+    from app_desktop import window_extrapolation_mixin, window_statistics_mixin
+
+    # inspect.getsource reads through linecache; drop stale entries a prior test
+    # may have left so getsource re-reads the current source (see #72).
+    linecache.clearcache()
+    refresh_src = inspect.getsource(window_mod.ExtrapolationWindow._refresh_display_format)
+    # The spec's kinds must be routed through the accessors, not re-hardcoded.
+    for kind in _RESULT_CSV_SPEC:
+        fname = result_csv_filename(kind)
+        # A bare filename literal next to _set_csv_data in the reformat method
+        # would mean the literal was re-hardcoded rather than sourced from spec.
+        assert f'suggestion="{fname}"' not in refresh_src, (
+            f"reformat path hardcodes {fname} instead of result_csv_filename({kind!r})"
+        )
+
+    extrap_src = inspect.getsource(window_extrapolation_mixin)
+    stats_src = inspect.getsource(window_statistics_mixin)
+    # The extrapolation/statistics first-render _show_* paths use the accessors.
+    assert 'result_csv_filename("extrapolation")' in extrap_src
+    assert 'result_csv_filename("statistics")' in stats_src