diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4540b34..1c1c694 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -47,8 +47,9 @@ jobs: pip install -e ".[dev]" - name: Run tests with coverage + timeout-minutes: 5 run: | - pytest --cov=pyqt_reactive --cov-report=xml --cov-report=html --cov-report=term-missing -v + pytest --cov=pyqt_reactive --cov-report=xml --cov-report=html --cov-report=term-missing -v -o faulthandler_timeout=120 - name: Upload coverage to Codecov if: matrix.os == 'ubuntu-latest' && matrix.python-version == '3.12' @@ -64,6 +65,8 @@ jobs: steps: - name: Checkout uses: actions/checkout@v4 + with: + fetch-depth: 0 - name: Setup Python uses: actions/setup-python@v5 @@ -73,17 +76,49 @@ jobs: - name: Install dependencies run: | python -m pip install --upgrade pip - pip install ruff black mypy + pip install -e ".[dev]" + + - name: Run full-package Ruff fatal checks + run: | + ruff check src/ --select E9,F63,F7,F822,F823 --output-format=github + + - name: Resolve quality diff base + id: quality-base + shell: bash + env: + EVENT_NAME: ${{ github.event_name }} + PR_BASE_SHA: ${{ github.event.pull_request.base.sha }} + PUSH_BASE_SHA: ${{ github.event.before }} + run: | + case "$EVENT_NAME" in + pull_request) base_sha="$PR_BASE_SHA" ;; + push) base_sha="$PUSH_BASE_SHA" ;; + *) base_sha="" ;; + esac + if [[ -z "$base_sha" || "$base_sha" =~ ^0+$ ]] || + ! git cat-file -e "${base_sha}^{commit}"; then + base_sha="$(git rev-parse HEAD^)" + fi + echo "sha=$base_sha" >> "$GITHUB_OUTPUT" + + - name: Reject Ruff diagnostics introduced on added lines + run: | + python scripts/check_ruff_added_lines.py \ + --base "${{ steps.quality-base.outputs.sha }}" \ + --head "$GITHUB_SHA" - - name: Run ruff (linting) + - name: Report repository-wide Ruff baseline (non-blocking) + continue-on-error: true run: | - ruff check src/ --output-format=github + ruff check src/ --statistics - - name: Run black (formatting check) + - name: Report repository-wide Black baseline (non-blocking) + continue-on-error: true run: | black --check src/ - - name: Run mypy (type checking) + - name: Report repository-wide Mypy baseline (non-blocking) + continue-on-error: true run: | mypy src/ --ignore-missing-imports diff --git a/CHANGELOG.md b/CHANGELOG.md index 850138d..1910737 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,19 @@ # Changelog +## [0.1.28] - 2026-07-30 + +### Changed + +- Reuse ObjectState's indexed nested-field topology instead of rescanning flat + paths for each form manager. +- Coalesce responsive-row construction into one layout transaction while + preserving reflow when group geometry changes. +- Resolve placeholders from canonical full ObjectState paths. + +### Dependencies + +- objectstate >= 1.0.21 + ## [0.1.0] - 2025-01-10 ### Added diff --git a/docs/index.rst b/docs/index.rst index 6624c72..6fbc666 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -72,7 +72,7 @@ Requirements * Python 3.11+ * PyQt6 >= 6.4.0 -* ObjectState >= 1.0.20 +* ObjectState >= 1.0.21 * python-introspect >= 0.1.6 * zmqruntime >= 0.1.19 diff --git a/pyproject.toml b/pyproject.toml index fc0fb80..e956c7f 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -4,7 +4,7 @@ build-backend = "hatchling.build" [project] name = "pyqt-reactive" -version = "0.1.27" +version = "0.1.28" description = "React-quality reactive form generation framework for PyQt6" authors = [{name = "Tristan Simas", email = "tristan.simas@mail.mcgill.ca"}] license = {text = "MIT"} @@ -29,7 +29,7 @@ dependencies = [ "magicgui>=0.7.0", "metaclass-registry>=0.1.5", "PyQt6>=6.4.0", - "objectstate>=1.0.20", + "objectstate>=1.0.21", "psutil>=5.9", "Pygments>=2.15", "pyzmq>=22.0", @@ -42,9 +42,9 @@ dev = [ "pytest>=7.0", "pytest-qt>=4.0", "pytest-cov>=4.0", - "black>=23.0", - "ruff>=0.1.0", - "mypy>=1.0", + "black==26.5.1", + "ruff==0.16.0", + "mypy==2.3.0", ] docs = [ "sphinx>=7.0", diff --git a/scripts/check_ruff_added_lines.py b/scripts/check_ruff_added_lines.py new file mode 100644 index 0000000..19159ba --- /dev/null +++ b/scripts/check_ruff_added_lines.py @@ -0,0 +1,155 @@ +#!/usr/bin/env python3 +"""Fail when configured Ruff diagnostics touch lines added by a Git diff.""" + +from __future__ import annotations + +import argparse +import json +import os +import re +import subprocess +import sys +from pathlib import Path + +HUNK_HEADER = re.compile(r"^@@ -\d+(?:,\d+)? \+(\d+)(?:,(\d+))? @@") + + +def _git_output(*arguments: str, binary: bool = False) -> str | bytes: + result = subprocess.run( + ("git", *arguments), + check=True, + capture_output=True, + text=not binary, + ) + return result.stdout + + +def _changed_python_files(base: str, head: str | None) -> tuple[Path, ...]: + revision_arguments = (base, head) if head is not None else (base,) + output = _git_output( + "diff", + "--name-only", + "--diff-filter=ACMR", + "-z", + *revision_arguments, + "--", + "*.py", + binary=True, + ) + assert isinstance(output, bytes) + return tuple(Path(os.fsdecode(path)) for path in output.split(b"\0") if path) + + +def _added_lines( + path: Path, + base: str, + head: str | None, +) -> frozenset[int]: + revision_arguments = (base, head) if head is not None else (base,) + output = _git_output( + "diff", + "--unified=0", + "--no-ext-diff", + "--no-color", + *revision_arguments, + "--", + os.fspath(path), + ) + assert isinstance(output, str) + added: set[int] = set() + for line in output.splitlines(): + match = HUNK_HEADER.match(line) + if match is None: + continue + first_line = int(match.group(1)) + line_count = int(match.group(2) or "1") + added.update(range(first_line, first_line + line_count)) + return frozenset(added) + + +def _ruff_diagnostics(paths: tuple[Path, ...]) -> list[dict[str, object]]: + result = subprocess.run( + ("ruff", "check", "--output-format=json", *(os.fspath(path) for path in paths)), + check=False, + capture_output=True, + text=True, + ) + try: + diagnostics = json.loads(result.stdout) + except json.JSONDecodeError as error: + raise RuntimeError( + "Ruff did not return JSON diagnostics.\n" + f"stdout:\n{result.stdout}\n" + f"stderr:\n{result.stderr}" + ) from error + if result.returncode not in {0, 1}: + raise RuntimeError( + f"Ruff failed with exit code {result.returncode}:\n{result.stderr}" + ) + return diagnostics + + +def _github_escape(value: object) -> str: + return ( + str(value) + .replace("%", "%25") + .replace("\r", "%0D") + .replace("\n", "%0A") + ) + + +def _relative_diagnostic_path(filename: object) -> Path: + path = Path(str(filename)) + if path.is_absolute(): + return path.relative_to(Path.cwd()) + return path + + +def main() -> int: + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument("--base", required=True, help="Git revision before the change") + parser.add_argument( + "--head", + help="Git revision after the change; omit to inspect the current worktree", + ) + arguments = parser.parse_args() + + changed_paths = _changed_python_files(arguments.base, arguments.head) + if not changed_paths: + print("No changed Python files.") + return 0 + + line_index = { + path: _added_lines(path, arguments.base, arguments.head) + for path in changed_paths + } + introduced = [] + for diagnostic in _ruff_diagnostics(changed_paths): + path = _relative_diagnostic_path(diagnostic["filename"]) + location = diagnostic["location"] + assert isinstance(location, dict) + row = int(location["row"]) + if row in line_index.get(path, frozenset()): + introduced.append((path, row, location, diagnostic)) + + if not introduced: + print( + "Configured Ruff rules report no diagnostics on added Python lines " + f"across {len(changed_paths)} changed files." + ) + return 0 + + for path, row, location, diagnostic in introduced: + code = diagnostic["code"] + message = diagnostic["message"] + print( + f"::error file={_github_escape(path)},line={row}," + f"col={location['column']},title=Ruff {code}::" + f"{_github_escape(message)}" + ) + print(f"{len(introduced)} Ruff diagnostics touch added Python lines.") + return 1 + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/src/pyqt_reactive/__init__.py b/src/pyqt_reactive/__init__.py index a394ff9..e05e120 100644 --- a/src/pyqt_reactive/__init__.py +++ b/src/pyqt_reactive/__init__.py @@ -19,7 +19,7 @@ - Cross-window reactive updates """ -__version__ = "0.1.27" +__version__ = "0.1.28" # Public API will be populated as modules are added __all__ = [ diff --git a/src/pyqt_reactive/forms/parameter_form_manager.py b/src/pyqt_reactive/forms/parameter_form_manager.py index 2e42fc4..ba38d9b 100644 --- a/src/pyqt_reactive/forms/parameter_form_manager.py +++ b/src/pyqt_reactive/forms/parameter_form_manager.py @@ -132,7 +132,7 @@ class ParameterFormTypeResolver: @staticmethod def field_type(manager: "ParameterFormManager", field_name: str, signature_type: Type) -> Type: dotted_path = f"{manager.field_id}.{field_name}" if manager.field_id else field_name - if not ParameterFormTypeResolver.path_has_children(manager.state.parameters, dotted_path): + if not manager.state.has_parameter_descendants(dotted_path): return signature_type state_type = manager.state.type_for_path(dotted_path) @@ -140,30 +140,16 @@ def field_type(manager: "ParameterFormManager", field_name: str, signature_type: return state_type return signature_type - @staticmethod - def path_has_children(parameters: Dict[str, Any], dotted_path: str) -> bool: - owner_path = DottedFieldPath(dotted_path) - return any(owner_path.contains_path(path) for path in parameters if path != dotted_path) - @staticmethod def scoped_parameters(state, field_id: str) -> ParameterDefaultsByName: """Project ObjectState's flat parameters into one form manager scope.""" - if not field_id: - return ParameterDefaultsByName( - (key, value) for key, value in state.parameters.items() if "." not in key + return ParameterDefaultsByName( + ( + parameter_path.field_name, + state.parameters[parameter_path.value], ) - - owner_path = DottedFieldPath(field_id) - result = ParameterDefaultsByName() - for path, value in state.parameters.items(): - if owner_path.contains_path(path) and path != field_id: - suffix = path[len(field_id) :] - if not suffix.startswith("."): - continue - remainder = suffix[1:] - if "." not in remainder: - result[remainder] = value - return result + for parameter_path in state.direct_parameter_paths(field_id) + ) class ParameterFormManager( diff --git a/src/pyqt_reactive/forms/widget_creation_config.py b/src/pyqt_reactive/forms/widget_creation_config.py index ae2c8d0..80ea508 100644 --- a/src/pyqt_reactive/forms/widget_creation_config.py +++ b/src/pyqt_reactive/forms/widget_creation_config.py @@ -1094,6 +1094,17 @@ def __init__(self, manager: ParameterFormManager, param_info: ParameterInfo) -> def run(self) -> QWidget: """Run the widget creation stages.""" self.create_container() + rt = self.runtime + if isinstance(rt.container, ResponsiveParameterRow): + with rt.container.layout_update(): + self._run_content_stages() + else: + self._run_content_stages() + return self.runtime.container + + def _run_content_stages(self) -> None: + """Build one row's content within its owner's layout transaction.""" + self.configure_nested_container() self.setup_layout() self.add_optional_title() @@ -1104,7 +1115,6 @@ def run(self) -> QWidget: self.connect_optional_checkbox() self.store_and_connect_widget() self.initialize_widget_semantics() - return self.runtime.container def create_container(self) -> None: rt = self.runtime diff --git a/src/pyqt_reactive/services/parameter_ops_service.py b/src/pyqt_reactive/services/parameter_ops_service.py index 4621e8f..a7ba62d 100644 --- a/src/pyqt_reactive/services/parameter_ops_service.py +++ b/src/pyqt_reactive/services/parameter_ops_service.py @@ -354,10 +354,9 @@ def refresh_all_placeholders(self, manager) -> None: if not PyQt6WidgetEnhancer.supports_placeholder(widget): continue - # Use manager.parameters (scoped) not state.parameters (full paths) - current_value = manager.parameters.get(param_name) - should_apply_placeholder = (current_value is None) full_path = f"{manager.field_id}.{param_name}" if manager.field_id else param_name + current_value = manager.state.parameters.get(full_path) + should_apply_placeholder = current_value is None resolved_value = manager.state.get_resolved_value(full_path) if should_apply_placeholder: diff --git a/src/pyqt_reactive/widgets/shared/responsive_layout_widgets.py b/src/pyqt_reactive/widgets/shared/responsive_layout_widgets.py index 7a34dd1..3311119 100644 --- a/src/pyqt_reactive/widgets/shared/responsive_layout_widgets.py +++ b/src/pyqt_reactive/widgets/shared/responsive_layout_widgets.py @@ -1,8 +1,9 @@ """Capacity-based responsive layout widgets for PyQt6.""" +from contextlib import contextmanager from enum import Enum -from PyQt6.QtCore import QSize, QTimer, Qt +from PyQt6.QtCore import QSize, Qt, QTimer from PyQt6.QtWidgets import ( QHBoxLayout, QLabel, @@ -113,6 +114,8 @@ def __init__(self, parent=None, layout_config=None): self._left_widgets: list[tuple[QWidget, int]] = [] self._right_widgets: list[tuple[QWidget, int]] = [] + self._layout_update_depth = 0 + self._layout_update_pending = False # Debounce timer self._timer = QTimer(self) @@ -122,14 +125,12 @@ def __init__(self, parent=None, layout_config=None): def add_left_widget(self, widget: QWidget, stretch: int = 0) -> None: """Add widget to left side (stays in row1).""" self._left_widgets.append((widget, stretch)) - self._do_switch() - self._timer.start(0) + self._request_layout_update() def add_right_widget(self, widget: QWidget, stretch: int = 0) -> None: """Add widget to right side (moves between row1 and row2).""" self._right_widgets.append((widget, stretch)) - self._do_switch() - self._timer.start(0) + self._request_layout_update() def release_widgets(self, *widgets: QWidget) -> bool: """Release widget ownership so later reflows cannot reinsert them.""" @@ -154,9 +155,29 @@ def retained( self._left_widgets = left_widgets self._right_widgets = right_widgets + self._request_layout_update() + return True + + @contextmanager + def layout_update(self): + """Coalesce one logical content update into one responsive reflow.""" + + self._layout_update_depth += 1 + try: + yield self + finally: + self._layout_update_depth -= 1 + if self._layout_update_depth == 0 and self._layout_update_pending: + self._layout_update_pending = False + self._do_switch() + self._timer.start(0) + + def _request_layout_update(self) -> None: + if self._layout_update_depth: + self._layout_update_pending = True + return self._do_switch() self._timer.start(0) - return True def is_empty(self) -> bool: """Return whether this row owns no remaining presentation widgets.""" @@ -291,6 +312,10 @@ def __init__(self, parent=None, spacing=4): self._last_row1 = [] self._last_row2 = [] self._last_width = -1 + self._last_group_layout_signature: tuple[ + tuple[str, int, int, bool, bool], + ..., + ] = () self._main_layout = QVBoxLayout(self) self._main_layout.setContentsMargins(0, 0, 0, 0) @@ -327,9 +352,6 @@ def set_groups(self, groups, stay_priority, right_align_names=None): self._groups = groups self._stay_priority = stay_priority self._right_align_names = set(right_align_names or []) - # Group contents and size policies can change while names and available - # width remain stable. Rebuild instead of reusing that stale row cache. - self._last_width = -1 self._update_layout() def refresh_layout(self): @@ -378,17 +400,29 @@ def _update_layout(self): row1_names = [name for name in visual_order if name in keep_names] row2_names = [name for name in visual_order if name not in keep_names] + group_layout_signature = tuple( + ( + name, + id(widget), + widths[name], + _widget_expands_horizontally(widget), + name in self._right_align_names, + ) + for name, widget in self._groups + ) if ( - available == self._last_width - and row1_names == self._last_row1 + row1_names == self._last_row1 and row2_names == self._last_row2 + and group_layout_signature == self._last_group_layout_signature ): + self._last_width = available return self._last_row1 = list(row1_names) self._last_row2 = list(row2_names) self._last_width = available + self._last_group_layout_signature = group_layout_signature group_map = {name: widget for name, widget in self._groups} diff --git a/tests/test_widgets.py b/tests/test_widgets.py index 97a2d2b..fcba5f4 100644 --- a/tests/test_widgets.py +++ b/tests/test_widgets.py @@ -268,6 +268,9 @@ class Component(Enum): parameters={"variable_components": [Component.SITE]}, config=SimpleNamespace(placeholder_prefix="Default"), state=SimpleNamespace( + parameters={ + "processing_config.variable_components": [Component.SITE], + }, get_resolved_value=lambda path: [Component.SITE], ), ) @@ -519,6 +522,104 @@ def test_responsive_parameter_row_wraps_only_below_minimum_capacity(qapp): assert reset.geometry().right() >= row.contentsRect().right() - row._row2_layout.contentsMargins().right() - 1 +def test_responsive_parameter_row_coalesces_logical_content_construction( + qapp, + monkeypatch, +): + """One row pipeline update performs one layout ownership transfer.""" + from PyQt6.QtWidgets import QLabel, QLineEdit, QPushButton + + from pyqt_reactive.widgets.shared.responsive_layout_widgets import ( + ResponsiveParameterRow, + ) + + row = ResponsiveParameterRow() + switch_calls = 0 + original_switch = row._do_switch + + def counted_switch() -> None: + nonlocal switch_calls + switch_calls += 1 + original_switch() + + monkeypatch.setattr(row, "_do_switch", counted_switch) + label = QLabel("Threshold") + editor = QLineEdit("1") + reset = QPushButton("Reset") + + with row.layout_update(): + row.set_label(label) + row.set_input(editor) + row.set_reset_button(reset) + + assert switch_calls == 1 + assert row._row1_layout.count() == 3 + assert row._row1_layout.itemAt(0).widget() is label + assert row._row1_layout.itemAt(1).widget() is editor + assert row._row1_layout.itemAt(2).widget() is reset + + +def test_staged_wrap_layout_keeps_unchanged_group_ownership(monkeypatch): + """Content growth does not repeatedly remove and reinsert stable groups.""" + from PyQt6.QtWidgets import QLabel, QWidget + + from pyqt_reactive.widgets.shared.responsive_layout_widgets import ( + StagedWrapLayout, + ) + + layout = StagedWrapLayout() + title_group = QWidget() + right_group = QWidget() + groups = [("title", title_group), ("right", right_group)] + layout.set_groups(groups, ["title", "right"]) + clear_calls = 0 + original_clear = layout._clear_row + + def counted_clear(row_layout) -> None: + nonlocal clear_calls + clear_calls += 1 + original_clear(row_layout) + + monkeypatch.setattr(layout, "_clear_row", counted_clear) + QLabel("new child", parent=right_group) + + layout.set_groups(groups, ["title", "right"]) + + assert clear_calls == 0 + assert layout._row1_layout.itemAt(0).widget() is title_group + assert layout._row1_layout.itemAt(1).widget() is right_group + + +def test_staged_wrap_layout_reclassifies_same_widget_after_intrinsic_growth(): + """A stable group identity still wraps when its required width changes.""" + from PyQt6.QtWidgets import QWidget + + from pyqt_reactive.widgets.shared.responsive_layout_widgets import ( + StagedWrapLayout, + ) + + layout = StagedWrapLayout() + layout.resize(200, 100) + title_group = QWidget() + title_group.setMinimumWidth(80) + right_group = QWidget() + right_group.setMinimumWidth(80) + groups = [("title", title_group), ("right", right_group)] + layout.set_groups(groups, ["title", "right"]) + + assert layout._row1_layout.itemAt(0).widget() is title_group + assert layout._row1_layout.itemAt(1).widget() is right_group + assert layout._row2_layout.count() == 0 + + right_group.setMinimumWidth(180) + layout.set_groups(groups, ["title", "right"]) + + assert layout._row1_layout.count() == 1 + assert layout._row1_layout.itemAt(0).widget() is title_group + assert layout._row2_layout.count() == 1 + assert layout._row2_layout.itemAt(0).widget() is right_group + + def test_action_tab_row_keeps_tabs_and_actions_on_one_row_when_they_fit(qapp): """Top-level tabs and active actions wrap only under horizontal pressure.""" from PyQt6.QtWidgets import QLabel, QPushButton