diff --git a/.github/hooks/run-validation-after-edits.json b/.github/hooks/run-validation-after-edits.json deleted file mode 100644 index 6371480a..00000000 --- a/.github/hooks/run-validation-after-edits.json +++ /dev/null @@ -1,11 +0,0 @@ -{ - "hooks": { - "PostToolUse": [ - { - "type": "command", - "command": "./.github/hooks/run_validation_after_edits.sh", - "timeout": 1800 - } - ] - } -} diff --git a/.github/hooks/run_validation_after_edits.sh b/.github/hooks/run_validation_after_edits.sh deleted file mode 100755 index 234f61aa..00000000 --- a/.github/hooks/run_validation_after_edits.sh +++ /dev/null @@ -1,92 +0,0 @@ -#!/usr/bin/env bash -set -euo pipefail - -payload="$(cat)" - -should_run="$(HOOK_PAYLOAD="$payload" python3 - <<'PY' -import json -import os - -edit_tools = { - "edit", - "write", - "multi_edit", - "multiedit", - "apply_patch", - "create_file", - "insert_edit_into_file", - "replace_string_in_file", - "multi_replace_string_in_file", - "edit_notebook_file", - "create_new_jupyter_notebook", - "mcp_github_create_or_update_file", - "mcp_io_github_git_create_or_update_file", -} - -# File extensions that require running the Python validation suite. -code_suffixes = (".py", ".toml", ".cfg", ".ini", ".yaml", ".yml", ".json") - -raw = os.environ.get("HOOK_PAYLOAD", "").strip() -if not raw: - print("skip") - raise SystemExit(0) - -try: - data = json.loads(raw) -except json.JSONDecodeError: - print("skip") - raise SystemExit(0) - -tool_name = str(data.get("tool_name") or data.get("toolName") or "").lower() -if tool_name not in edit_tools: - print("skip") - raise SystemExit(0) - - -def paths(value): - """Yield path-like strings from tool input.""" - if isinstance(value, dict): - for key, item in value.items(): - if key in {"filePath", "file_path", "path", "notebookPath"} and isinstance( - item, str - ): - yield item - else: - yield from paths(item) - elif isinstance(value, list): - for item in value: - yield from paths(item) - - -tool_input = data.get("tool_input") or data.get("toolInput") or {} -edited = list(paths(tool_input)) - -# Only run the heavy validation when code/config files were touched. -# Docs, markdown, and memory edits do not need pytest + prek. -if edited and not any(p.lower().endswith(code_suffixes) for p in edited): - print("skip") - raise SystemExit(0) - -print("run") -PY -)" - -if [[ "$should_run" != "run" ]]; then - exit 0 -fi - -echo "[hook] File edit detected. Running validation commands..." - -echo "[hook] Running tests" -if ! uv run pytest --no-cov; then - echo "[hook] Tests failed" - exit 2 -fi - -echo "[hook] Running prek" -if ! SKIP=no-commit-to-branch uv run prek run --all-files; then - echo "[hook] Prek failed" - exit 2 -fi - -echo "[hook] Validation passed" diff --git a/openspec/changes/archive/2026-07-16-fix-partial-validation-state/.openspec.yaml b/openspec/changes/archive/2026-07-16-fix-partial-validation-state/.openspec.yaml new file mode 100644 index 00000000..0fd3f8ab --- /dev/null +++ b/openspec/changes/archive/2026-07-16-fix-partial-validation-state/.openspec.yaml @@ -0,0 +1,3 @@ +--- +schema: spec-driven +created: 2026-07-16 diff --git a/openspec/changes/archive/2026-07-16-fix-partial-validation-state/design.md b/openspec/changes/archive/2026-07-16-fix-partial-validation-state/design.md new file mode 100644 index 00000000..f01bb4cd --- /dev/null +++ b/openspec/changes/archive/2026-07-16-fix-partial-validation-state/design.md @@ -0,0 +1,86 @@ +## Context + +`SectionValidator` lazily validates device parameter support before a section or +hot-water group is read. It currently stores completion as a section-wide set in +`APIValidator` and a group-wide set in `SectionValidator`. An `include` request +validates only a subset but records that coarse completion state, so a later +request for different names or all names can skip validation. + +The resulting configuration has not established whether every requested +parameter is supported. For normal sections, an unvalidated parameter can remain +in the configuration and later produce a strict response-mapping failure. For +hot water, the cache contains only the first subset and later reads silently use +that incomplete cache. + +## Goals / Non-Goals + +**Goals:** + +- Record lazy-validation coverage by parameter ID for sections and hot-water + groups. +- Revalidate only the parameter IDs required by a later request when they have + not already been covered. +- Retain the existing public method signatures, include-filter validation, and + per-section/group locking model. +- Add regression tests for sequential filtered and unfiltered access. + +**Non-Goals:** + +- Change the public `include` API or make it accept parameter IDs. +- Alter device-response parsing, model serialization, or unsupported-parameter + rules. +- Retry or cache failed validation requests. + +## Decisions + +### Track covered parameter IDs per validation owner + +`APIValidator` will maintain a mapping from section name to the set of parameter +IDs whose support was checked. `SectionValidator` will maintain the equivalent +mapping for hot-water group names. Completion for a request is determined by +whether its requested IDs are a subset of that owner’s covered IDs. + +Parameter IDs are chosen over parameter names because they are the identifiers +sent to BSB-LAN and remain unambiguous if a configuration ever aliases a name. +When a response establishes an ID is unsupported, it is removed from the active +configuration/cache but remains covered, preventing needless repeated probes. + +Alternative considered: avoid persisting any completion after an `include` +request. This avoids the defect but discards useful knowledge and repeatedly +validates already checked parameters. Parameter-level coverage keeps lazy +loading efficient while remaining correct. + +### Derive requested IDs before the one-time gate + +Each ensure method will first resolve the configured IDs for its section or +group, applying `include` when present. Its lock’s fast and post-lock checks will +compare that specific requested-ID set to coverage rather than use a coarse +section/group marker. A full request resolves all currently configured IDs and +therefore completes only after each is covered. + +An include filter that matches no IDs will not add coverage or mark a group or +section complete. Existing downstream include validation remains responsible for +returning the appropriate invalid-include error. + +Alternative considered: retain a boolean for full validation plus a separate +partial set. This duplicates state and makes configuration changes harder to +reason about; one coverage mapping expresses both cases. + +### Preserve locks and recheck coverage after waiting + +The existing per-section and per-group locks remain in place. The completion +predicate will calculate coverage for the current requested IDs both before and +after lock acquisition. Concurrent calls requesting different subsets therefore +serialize safely, and the second call still validates its uncovered IDs. + +## Risks / Trade-offs + +- [Configuration mutation during validation] → Derive the requested IDs from + the current configuration at each completion check; removed unsupported IDs + no longer need validation. +- [Stale internal tests inspect completion sets] → Update tests to assert + parameter coverage and externally observable request/cache behavior instead + of obsolete whole-group completion. +- [Additional first-read requests] → A later request for previously unrequested + parameters necessarily makes one validation request; already covered IDs are + not re-requested. diff --git a/openspec/changes/archive/2026-07-16-fix-partial-validation-state/proposal.md b/openspec/changes/archive/2026-07-16-fix-partial-validation-state/proposal.md new file mode 100644 index 00000000..797bd295 --- /dev/null +++ b/openspec/changes/archive/2026-07-16-fix-partial-validation-state/proposal.md @@ -0,0 +1,38 @@ +## Why + +Lazy validation currently records a section or hot-water group as fully +validated after a request filtered with `include`. This can leave unsupported +parameters in a section or leave the hot-water cache incomplete, causing later +unfiltered reads to skip necessary validation and return incomplete data or fail +while mapping a device response. + +## What Changes + +- Track validation coverage at the parameter level for sections and hot-water + groups instead of treating any successful filtered request as complete. +- Ensure a later request validates parameter names that an earlier `include` + request did not cover. +- Keep validation completion correct when an include filter has no matching + parameters. +- Add focused regression coverage for filtered-then-unfiltered and + differently-filtered reads. + +## Capabilities + +### New Capabilities + +- `lazy-validation-completeness`: Lazy section and hot-water validation covers + every parameter required by each request before that request uses the cached + configuration. + +### Modified Capabilities + +- None. + +## Impact + +- Affected code: `src/bsblan/_validation.py`, `src/bsblan/utility.py`, and the + lazy section and hot-water read paths that use their validation state. +- Affected tests: validation, include-filter, and hot-water regression tests. +- Public API: no signature changes; `include` calls may perform an additional + validation request when they need parameters not checked by a prior call. diff --git a/openspec/changes/archive/2026-07-16-fix-partial-validation-state/specs/lazy-validation-completeness/spec.md b/openspec/changes/archive/2026-07-16-fix-partial-validation-state/specs/lazy-validation-completeness/spec.md new file mode 100644 index 00000000..83dbaa54 --- /dev/null +++ b/openspec/changes/archive/2026-07-16-fix-partial-validation-state/specs/lazy-validation-completeness/spec.md @@ -0,0 +1,63 @@ +## ADDED Requirements + +### Requirement: Section validation covers every requested parameter + +Before a lazy section read uses its configured parameters, the client SHALL +validate support for every parameter ID required by that read. A prior validation +of a different included subset SHALL NOT make unvalidated parameter IDs appear +validated. The client SHALL not repeat validation for IDs already covered unless +validation state is explicitly reset. + +#### Scenario: Filtered section read followed by unfiltered read + +- **WHEN** a section is first read with `include` selecting one parameter and a + later read requests the complete section +- **THEN** the later read validates every configured parameter ID not checked by + the first read before fetching the complete section + +#### Scenario: Differently filtered section read + +- **WHEN** a section is read with one included parameter and later read with a + different included parameter +- **THEN** the second read validates the newly requested parameter ID +- **AND** it does not revalidate the parameter ID already covered by the first + read + +#### Scenario: Unsupported parameter discovered by a later read + +- **WHEN** a later section read validates a previously uncovered parameter ID + that the device does not support +- **THEN** the client removes that parameter from the active configuration +- **AND** subsequent reads do not request that unsupported parameter + +### Requirement: Hot-water group validation covers every requested parameter + +Before a hot-water group read uses its cached parameters, the client SHALL +validate and cache every parameter ID required by that read. A prior validation +of an included subset SHALL NOT make the group complete for other parameter IDs. + +#### Scenario: Filtered hot-water read followed by complete group read + +- **WHEN** a hot-water group is first read with `include` selecting a subset and + later read without `include` +- **THEN** the later read validates and caches every remaining supported + parameter ID in that group before it fetches group data + +#### Scenario: Differently filtered hot-water read + +- **WHEN** a hot-water group is read with one included parameter and later read + with a different included parameter +- **THEN** the second read validates and caches the newly requested parameter ID +- **AND** it does not revalidate the parameter ID already covered by the first + read + +### Requirement: Empty include results do not complete validation + +When an include filter selects no parameter IDs in a section or hot-water group, +the client SHALL NOT record that owner as complete or add parameter coverage. + +#### Scenario: Later valid read after no-match include + +- **WHEN** an include filter matches no parameters and a later request selects + one or more valid parameters from the same section or hot-water group +- **THEN** the later request validates its selected parameter IDs normally diff --git a/openspec/changes/archive/2026-07-16-fix-partial-validation-state/tasks.md b/openspec/changes/archive/2026-07-16-fix-partial-validation-state/tasks.md new file mode 100644 index 00000000..803aae39 --- /dev/null +++ b/openspec/changes/archive/2026-07-16-fix-partial-validation-state/tasks.md @@ -0,0 +1,29 @@ +## 1. Parameter-Level Validation Coverage + +- [x] 1.1 Replace coarse section completion tracking in `APIValidator` with + per-section parameter-ID coverage, including reset behavior. +- [x] 1.2 Update lazy section validation to derive the current requested IDs, + gate on their coverage, and retain unsupported-ID removal behavior. +- [x] 1.3 Replace coarse hot-water group completion tracking with per-group + parameter-ID coverage and keep the cache synchronized with validated IDs. +- [x] 1.4 Ensure empty include-filter results add no coverage and do not mark a + section or hot-water group complete. + +## 2. Regression Coverage + +- [x] 2.1 Add section tests for filtered-then-unfiltered and + differently-filtered reads, asserting uncovered IDs are validated once. +- [x] 2.2 Add a section test showing a later-discovered unsupported ID is + removed before the complete read is mapped. +- [x] 2.3 Update hot-water include tests to assert a partial request does not + complete the group, then add filtered-then-unfiltered and + differently-filtered regression cases. +- [x] 2.4 Update the empty include-filter test to assert no completion state is + recorded and a later valid request performs validation. +- [x] 2.5 Retain or extend locking coverage to prove sequentially serialized + requests recheck their own parameter coverage. + +## 3. Validation + +- [x] 3.1 Run focused validation and include-filter tests with `--no-cov`. +- [x] 3.2 Run `uv run prek run --all-files` and resolve all reported failures. diff --git a/openspec/specs/lazy-validation-completeness/spec.md b/openspec/specs/lazy-validation-completeness/spec.md new file mode 100644 index 00000000..e6b8641f --- /dev/null +++ b/openspec/specs/lazy-validation-completeness/spec.md @@ -0,0 +1,71 @@ +# lazy-validation-completeness + +## Purpose + +Lazy section and hot-water validation must record the parameter IDs it has +actually checked so filtered reads cannot suppress validation required by later +requests. + +## Requirements + +### Requirement: Section validation covers every requested parameter + +Before a lazy section read uses its configured parameters, the client SHALL +validate support for every parameter ID required by that read. A prior validation +of a different included subset SHALL NOT make unvalidated parameter IDs appear +validated. The client SHALL not repeat validation for IDs already covered unless +validation state is explicitly reset. + +#### Scenario: Filtered section read followed by unfiltered read + +- **WHEN** a section is first read with `include` selecting one parameter and a + later read requests the complete section +- **THEN** the later read validates every configured parameter ID not checked by + the first read before fetching the complete section + +#### Scenario: Differently filtered section read + +- **WHEN** a section is read with one included parameter and later read with a + different included parameter +- **THEN** the second read validates the newly requested parameter ID +- **AND** it does not revalidate the parameter ID already covered by the first + read + +#### Scenario: Unsupported parameter discovered by a later read + +- **WHEN** a later section read validates a previously uncovered parameter ID + that the device does not support +- **THEN** the client removes that parameter from the active configuration +- **AND** subsequent reads do not request that unsupported parameter + +### Requirement: Hot-water group validation covers every requested parameter + +Before a hot-water group read uses its cached parameters, the client SHALL +validate and cache every parameter ID required by that read. A prior validation +of an included subset SHALL NOT make the group complete for other parameter IDs. + +#### Scenario: Filtered hot-water read followed by complete group read + +- **WHEN** a hot-water group is first read with `include` selecting a subset and + later read without `include` +- **THEN** the later read validates and caches every remaining supported + parameter ID in that group before it fetches group data + +#### Scenario: Differently filtered hot-water read + +- **WHEN** a hot-water group is read with one included parameter and later read + with a different included parameter +- **THEN** the second read validates and caches the newly requested parameter ID +- **AND** it does not revalidate the parameter ID already covered by the first + read + +### Requirement: Empty include results do not complete validation + +When an include filter selects no parameter IDs in a section or hot-water group, +the client SHALL NOT record that owner as complete or add parameter coverage. + +#### Scenario: Later valid read after no-match include + +- **WHEN** an include filter matches no parameters and a later request selects + one or more valid parameters from the same section or hot-water group +- **THEN** the later request validates its selected parameter IDs normally diff --git a/src/bsblan/_validation.py b/src/bsblan/_validation.py index 4f756505..cc0b93d2 100644 --- a/src/bsblan/_validation.py +++ b/src/bsblan/_validation.py @@ -75,8 +75,9 @@ def __init__( self._api_validator: APIValidator | None = None # Cache of validated hot water parameter id -> name. self._hot_water_param_cache: dict[str, str] = {} - # Track which hot water param groups have been validated. + # Track which hot water param groups have been fully validated. self._validated_hot_water_groups: set[str] = set() + self._validated_hot_water_parameters: dict[str, set[str]] = {} # Locks to prevent concurrent validation of the same section/group. self._section_locks: dict[str, asyncio.Lock] = {} self._hot_water_group_locks: dict[str, asyncio.Lock] = {} @@ -192,6 +193,11 @@ async def ensure_section_validated( raise BSBLANError(ErrorMsg.API_VALIDATOR_NOT_INITIALIZED) api_validator = self._api_validator + if api_validator.is_section_validated(section): + return + + def _requested_parameter_ids() -> set[str]: + return set(self._get_section_params(section, include)) async def _validate() -> None: logger.debug("Lazy loading section: %s", section) @@ -205,7 +211,9 @@ async def _validate() -> None: await self._run_once_locked( section, self._section_locks, - lambda: api_validator.is_section_validated(section), + lambda: api_validator.are_parameters_validated( + section, _requested_parameter_ids() + ), _validate, ) @@ -231,49 +239,38 @@ async def ensure_hot_water_group_validated( If provided, only these parameters will be validated. """ + if group_name in self._validated_hot_water_groups: + return - async def _validate() -> None: - if not self._api_validator: - raise BSBLANError(ErrorMsg.API_VALIDATOR_NOT_INITIALIZED) + def _group_params() -> dict[str, str]: + return self._get_hot_water_group_params(param_filter, include) - api_data = self._get_api_data() - if not api_data: - raise BSBLANError(ErrorMsg.API_DATA_NOT_INITIALIZED) + def _requested_parameter_ids() -> set[str]: + return set(_group_params()) + async def _validate() -> None: logger.debug("Lazy loading hot water group: %s", group_name) - - # Get the base hot water params from API config - section_data = api_data.get("hot_water", {}) - - # Filter to only the params in this group - group_params = { - param_id: param_name - for param_id, param_name in section_data.items() - if param_id in param_filter + group_params = _group_params() + uncovered_params = { + param_id: name + for param_id, name in group_params.items() + if param_id + not in self._validated_hot_water_parameters.get(group_name, set()) } - # Apply include filter if specified - only validate requested params - if include is not None: - group_params = { - param_id: name - for param_id, name in group_params.items() - if name in include - } - - if not group_params: + if not uncovered_params: logger.debug("No parameters to validate for group %s", group_name) - self._validated_hot_water_groups.add(group_name) return # Request only these specific parameters from the device - params = self._extract_params_summary(group_params) + params = self._extract_params_summary(uncovered_params) response_data = await self._request( params={"Parameter": params["string_par"]} ) # Validate and filter out unsupported params params_to_remove = [] - for param_id, param_name in group_params.items(): + for param_id, param_name in uncovered_params.items(): if param_id not in response_data: logger.info( "Hot water param %s (%s) not found in response", @@ -294,26 +291,87 @@ async def _validate() -> None: params_to_remove.append(param_id) # Update the cache with validated params for this group - for param_id, param_name in group_params.items(): + for param_id, param_name in uncovered_params.items(): if param_id not in params_to_remove: self._hot_water_param_cache[param_id] = param_name - # Mark this group as validated - self._validated_hot_water_groups.add(group_name) + validated_ids = self._validated_hot_water_parameters.setdefault( + group_name, set() + ) + validated_ids.update(uncovered_params) + if ( + self._get_hot_water_group_params(param_filter, None).keys() + <= validated_ids + ): + self._validated_hot_water_groups.add(group_name) + else: + self._validated_hot_water_groups.discard(group_name) logger.debug( "Validated hot water group '%s': %d params, removed %d unsupported", group_name, - len(group_params), + len(uncovered_params), len(params_to_remove), ) await self._run_once_locked( group_name, self._hot_water_group_locks, - lambda: group_name in self._validated_hot_water_groups, + lambda: self._are_hot_water_parameters_validated( + group_name, _requested_parameter_ids() + ), _validate, ) + def _get_section_params( + self, section: SectionLiteral, include: list[str] | None + ) -> dict[str, str]: + """Get the configured parameters selected for a section request.""" + api_data = self._get_api_data() + if not api_data: + raise BSBLANError(ErrorMsg.API_DATA_NOT_INITIALIZED) + + try: + section_data = api_data[section] + except KeyError as err: + msg = ErrorMsg.SECTION_NOT_FOUND.format(section) + raise BSBLANError(msg) from err + + if include is None: + return section_data + return { + param_id: name for param_id, name in section_data.items() if name in include + } + + def _get_hot_water_group_params( + self, param_filter: set[str], include: list[str] | None + ) -> dict[str, str]: + """Get configured parameters selected for a hot water group request.""" + if not self._api_validator: + raise BSBLANError(ErrorMsg.API_VALIDATOR_NOT_INITIALIZED) + + api_data = self._get_api_data() + if not api_data: + raise BSBLANError(ErrorMsg.API_DATA_NOT_INITIALIZED) + + group_params = { + param_id: param_name + for param_id, param_name in api_data.get("hot_water", {}).items() + if param_id in param_filter + } + if include is None: + return group_params + return { + param_id: name for param_id, name in group_params.items() if name in include + } + + def _are_hot_water_parameters_validated( + self, group_name: str, parameter_ids: set[str] + ) -> bool: + """Check whether every requested hot water parameter ID was validated.""" + return group_name in self._validated_hot_water_groups or parameter_ids.issubset( + self._validated_hot_water_parameters.get(group_name, set()) + ) + async def _validate_api_section( self, section: SectionLiteral, include: list[str] | None = None ) -> dict[str, Any] | None: @@ -340,36 +398,31 @@ async def _validate_api_section( if not api_data: raise BSBLANError(ErrorMsg.API_DATA_NOT_INITIALIZED) - # Assign to local variable after asserting it's not None api_validator = self._api_validator - if api_validator.is_section_validated(section): return None - # Get parameters for the section - try: - section_data = api_data[section] - except KeyError as err: - msg = ErrorMsg.SECTION_NOT_FOUND.format(section) - raise BSBLANError(msg) from err + section_data = self._get_section_params(section, include) + uncovered_params = { + param_id: name + for param_id, name in section_data.items() + if not api_validator.are_parameters_validated(section, {param_id}) + } - # Filter to only included params if specified - if include is not None: - section_data = { - param_id: name - for param_id, name in section_data.items() - if name in include - } + if not uncovered_params: + return None try: # Request data from device for validation - params = self._extract_params_summary(section_data) + params = self._extract_params_summary(uncovered_params) response_data = await self._request( params={"Parameter": params["string_par"]} ) # Validate the section against actual device response - api_validator.validate_section(section, response_data, include) + api_validator.validate_section( + section, response_data, parameter_ids=set(uncovered_params) + ) # Update API data with validated configuration api_data[section] = api_validator.get_section_params(section) diff --git a/src/bsblan/utility.py b/src/bsblan/utility.py index e7b0701e..89a95862 100644 --- a/src/bsblan/utility.py +++ b/src/bsblan/utility.py @@ -94,12 +94,14 @@ class APIValidator: # Flexible type for API data (accepts `APIConfig`, plain dicts, or None) api_config: Any # intentionally permissive to support tests and dynamic data validated_sections: set[str] = field(default_factory=set) + validated_parameters: dict[str, set[str]] = field(default_factory=dict) def validate_section( self, section: str, request_data: dict[str, Any], include: list[str] | None = None, + parameter_ids: set[str] | None = None, ) -> None: """Validate and update a section of API config based on actual device support. @@ -109,6 +111,8 @@ def validate_section( request_data: Response data from the device for validation include: Optional list of parameter names to validate. If None, validates all parameters for the section. + parameter_ids: Optional parameter IDs to validate. When supplied, + takes precedence over ``include``. """ # Check if the section exists in the APIConfig object @@ -116,19 +120,27 @@ def validate_section( logger.warning("Unknown section '%s' in API configuration", section) return - # Skip if section was already validated if section in self.validated_sections: logger.debug("Section '%s' was already validated", section) return section_config = self.api_config[section] params_to_remove = [] + validated_ids = set() # Check each parameter in the section (filtered by include if specified) - for param_id, param_name in section_config.items(): + for param_id, param_name in list(section_config.items()): # Skip params not in include list (if include is specified) - if include is not None and param_name not in include: + if parameter_ids is not None and param_id not in parameter_ids: continue + if ( + parameter_ids is None + and include is not None + and param_name not in include + ): + continue + + validated_ids.add(param_id) if param_id not in request_data: logger.info( @@ -153,8 +165,11 @@ def validate_section( for param_id in params_to_remove: section_config.pop(param_id) - # Mark section as validated - self.validated_sections.add(section) + self.validated_parameters.setdefault(section, set()).update(validated_ids) + if set(section_config).issubset(self.validated_parameters[section]): + self.validated_sections.add(section) + else: + self.validated_sections.discard(section) logger.debug( "Validated section '%s': removed %d unsupported parameters", @@ -170,8 +185,14 @@ def get_section_params(self, section: str) -> Any: """Get the parameter mapping for a section.""" return (self.api_config or {}).get(section, {}).copy() + def are_parameters_validated(self, section: str, parameter_ids: set[str]) -> bool: + """Check whether every requested parameter ID was validated.""" + return section in self.validated_sections or parameter_ids.issubset( + self.validated_parameters.get(section, set()) + ) + def is_section_validated(self, section: str) -> bool: - """Check if a section has been validated.""" + """Check whether every current parameter in a section was validated.""" return section in self.validated_sections def reset_validation(self, section: str | None = None) -> None: @@ -183,5 +204,9 @@ def reset_validation(self, section: str | None = None) -> None: """ if section is None: self.validated_sections.clear() + self.validated_parameters.clear() elif section in self.validated_sections: self.validated_sections.remove(section) + self.validated_parameters.pop(section, None) + else: + self.validated_parameters.pop(section, None) diff --git a/tests/test_api_validation.py b/tests/test_api_validation.py index 121cb0ab..80bbea3d 100644 --- a/tests/test_api_validation.py +++ b/tests/test_api_validation.py @@ -115,6 +115,17 @@ async def test_validate_api_section_no_api_data() -> None: await bsblan._validator._validate_api_section("device") +@pytest.mark.asyncio +async def test_get_section_params_no_api_data() -> None: + """Test selected section parameter lookup without API data.""" + async with aiohttp.ClientSession() as session: + bsblan = BSBLAN(BSBLANConfig(host="example.com"), session=session) + bsblan._api_data = None + + with pytest.raises(BSBLANError, match=ErrorMsg.API_DATA_NOT_INITIALIZED): + bsblan._validator._get_section_params("device", None) + + @pytest.mark.asyncio async def test_validate_api_section_invalid_section() -> None: """Test API section validation with invalid section.""" @@ -178,6 +189,7 @@ def mock_validate( _section: str, _response: dict[str, Any], _include: list[str] | None = None, + **_kwargs: Any, ) -> NoReturn: error_message = "Validation error" raise BSBLANError(error_message) @@ -234,6 +246,19 @@ async def test_validate_section_already_validated(monkeypatch: Any) -> None: assert response_data is None +@pytest.mark.asyncio +async def test_validate_api_section_skips_covered_parameters() -> None: + """Test section validation returns when every selected ID is covered.""" + async with aiohttp.ClientSession() as session: + bsblan = BSBLAN(BSBLANConfig(host="example.com"), session=session) + bsblan._supports_full_config = True + bsblan._api_data = {"heating": {"700": "operating_mode"}} # type: ignore[assignment] + bsblan._validator._api_validator = APIValidator(bsblan._api_data) + bsblan._validator._api_validator.validated_parameters["heating"] = {"700"} + + assert await bsblan._validator._validate_api_section("heating") is None + + @pytest.mark.asyncio async def test_validation_error_resets_section(monkeypatch: Any) -> None: """Test that validation errors reset the section (line 212).""" @@ -370,6 +395,111 @@ async def slow_validate( assert validation_count == 1 +@pytest.mark.asyncio +async def test_ensure_section_validated_validates_uncovered_params_once() -> None: + """Test a complete read validates only IDs missed by an include read.""" + async with aiohttp.ClientSession() as session: + bsblan = BSBLAN(BSBLANConfig(host="example.com"), session=session) + bsblan._supports_full_config = True + bsblan._api_data = { # type: ignore[assignment] + "heating": { + "700": "operating_mode", + "710": "target_temperature", + } + } + bsblan._validator._api_validator = APIValidator(bsblan._api_data) + + requests: list[set[str]] = [] + + async def mock_request( + params: dict[str, str] | None = None, + **_kwargs: Any, + ) -> dict[str, Any]: + param_ids = set((params or {})["Parameter"].split(",")) + requests.append(param_ids) + return {param_id: {"value": "1", "unit": ""} for param_id in param_ids} + + bsblan._request = mock_request # type: ignore[method-assign] + + await bsblan._ensure_section_validated("heating", include=["operating_mode"]) + await bsblan._ensure_section_validated("heating") + await bsblan._ensure_section_validated("heating") + + assert requests == [{"700"}, {"710"}] + + +@pytest.mark.asyncio +async def test_ensure_section_validated_validates_different_include() -> None: + """Test a different include validates the newly requested ID.""" + async with aiohttp.ClientSession() as session: + bsblan = BSBLAN(BSBLANConfig(host="example.com"), session=session) + bsblan._supports_full_config = True + bsblan._api_data = { # type: ignore[assignment] + "heating": { + "700": "operating_mode", + "710": "target_temperature", + } + } + bsblan._validator._api_validator = APIValidator(bsblan._api_data) + + requests: list[set[str]] = [] + + async def mock_request( + params: dict[str, str] | None = None, + **_kwargs: Any, + ) -> dict[str, Any]: + param_ids = set((params or {})["Parameter"].split(",")) + requests.append(param_ids) + return {param_id: {"value": "1", "unit": ""} for param_id in param_ids} + + bsblan._request = mock_request # type: ignore[method-assign] + + await bsblan._ensure_section_validated("heating", include=["operating_mode"]) + await bsblan._ensure_section_validated( + "heating", include=["target_temperature"] + ) + + assert requests == [{"700"}, {"710"}] + + +@pytest.mark.asyncio +async def test_ensure_section_validated_removes_later_unsupported_param() -> None: + """Test a later validation removes an unsupported parameter before reads.""" + async with aiohttp.ClientSession() as session: + bsblan = BSBLAN(BSBLANConfig(host="example.com"), session=session) + bsblan._supports_full_config = True + bsblan._api_data = { # type: ignore[assignment] + "heating": { + "700": "operating_mode", + "710": "target_temperature", + } + } + bsblan._validator._api_validator = APIValidator(bsblan._api_data) + + requests: list[set[str]] = [] + + async def mock_request( + params: dict[str, str] | None = None, + **_kwargs: Any, + ) -> dict[str, Any]: + param_ids = set((params or {})["Parameter"].split(",")) + requests.append(param_ids) + if "700" in param_ids: + return {"700": {"value": "1", "unit": ""}} + return {} + + bsblan._request = mock_request # type: ignore[method-assign] + + await bsblan._ensure_section_validated("heating", include=["operating_mode"]) + await bsblan._ensure_section_validated("heating") + await bsblan._ensure_section_validated("heating") + + assert requests == [{"700"}, {"710"}] + assert bsblan._validator.get_section_params("heating") == { + "700": "operating_mode" + } + + @pytest.mark.asyncio async def test_validate_api_section_hot_water_cache() -> None: """Test that hot_water section validation populates the cache.""" diff --git a/tests/test_hot_water_additional.py b/tests/test_hot_water_additional.py index c8b5611b..0a6e0671 100644 --- a/tests/test_hot_water_additional.py +++ b/tests/test_hot_water_additional.py @@ -348,7 +348,7 @@ def mock_request(**kwargs: Any) -> dict[str, Any]: async def test_granular_validation_empty_params( monkeypatch: Any, ) -> None: - """Test granular validation when no params match filter.""" + """Test granular validation leaves an empty group incomplete.""" async with aiohttp.ClientSession() as session: config = BSBLANConfig(host="example.com") bsblan = BSBLAN(config, session=session) @@ -362,11 +362,11 @@ async def test_granular_validation_empty_params( api_validator = APIValidator(api_data) bsblan._validator._api_validator = api_validator - # Validation should complete without error even with empty params + # Validation should complete without error even with empty params. await bsblan._ensure_hot_water_group_validated("essential", {"1600", "1610"}) - # Group should be marked as validated - assert "essential" in bsblan._validator._validated_hot_water_groups + # No parameter coverage means the group is not complete. + assert "essential" not in bsblan._validator._validated_hot_water_groups @pytest.mark.asyncio @@ -541,6 +541,30 @@ async def test_ensure_hot_water_group_double_check_after_lock() -> None: bsblan._request.assert_not_awaited() # type: ignore[attr-defined] +@pytest.mark.asyncio +async def test_ensure_hot_water_group_skips_recheck_without_uncovered_params( + monkeypatch: Any, +) -> None: + """Test the locked recheck skips a group whose IDs are already covered.""" + async with aiohttp.ClientSession() as session: + bsblan = BSBLAN(BSBLANConfig(host="example.com"), session=session) + bsblan._supports_full_config = True + bsblan._api_data = {"hot_water": {"1600": "operating_mode"}} # type: ignore[assignment] + bsblan._validator._api_validator = APIValidator(bsblan._api_data) + bsblan._validator._validated_hot_water_parameters["essential"] = {"1600"} + bsblan._request = AsyncMock() # type: ignore[method-assign] + + monkeypatch.setattr( + bsblan._validator, + "_are_hot_water_parameters_validated", + lambda _group_name, _parameter_ids: False, + ) + + await bsblan._ensure_hot_water_group_validated("essential", {"1600"}) + + bsblan._request.assert_not_awaited() # type: ignore[attr-defined] + + @pytest.mark.asyncio async def test_ensure_hot_water_group_concurrent_double_check() -> None: """Test that concurrent hot water group validation doesn't duplicate.""" @@ -632,7 +656,7 @@ async def mock_request( @pytest.mark.asyncio async def test_ensure_hot_water_group_validated_include_empty_result() -> None: - """Test that include filter with no matching params marks group validated.""" + """Test that include filter with no matching params leaves group incomplete.""" async with aiohttp.ClientSession() as session: bsblan = BSBLAN(BSBLANConfig(host="example.com"), session=session) bsblan._supports_full_config = True @@ -645,9 +669,14 @@ async def test_ensure_hot_water_group_validated_include_empty_result() -> None: request_count = 0 - async def mock_request(**_kwargs: Any) -> dict[str, Any]: + async def mock_request( + params: dict[str, str] | None = None, + **_kwargs: Any, + ) -> dict[str, Any]: nonlocal request_count request_count += 1 + if params and params.get("Parameter") == "1640": + return {"1640": {"value": "1", "unit": ""}} return {} bsblan._request = mock_request # type: ignore[method-assign] @@ -661,8 +690,17 @@ async def mock_request(**_kwargs: Any) -> dict[str, Any]: # No request should be made since no params match assert request_count == 0 - # Group should still be marked as validated - assert "config" in bsblan._validator._validated_hot_water_groups + # No matching parameters means no coverage or completion state. + assert "config" not in bsblan._validator._validated_hot_water_groups + + await bsblan._ensure_hot_water_group_validated( + "config", + {"1640"}, + include=["legionella_function"], + ) + + assert request_count == 1 + assert "1640" in bsblan._validator._hot_water_param_cache @pytest.mark.asyncio @@ -712,3 +750,140 @@ async def mock_request( assert "1640" in bsblan._validator._hot_water_param_cache assert "1645" in bsblan._validator._hot_water_param_cache assert "1648" in bsblan._validator._hot_water_param_cache + + +@pytest.mark.asyncio +async def test_ensure_hot_water_group_validated_loads_uncovered_params() -> None: + """Test a complete group read validates IDs missed by an include read.""" + async with aiohttp.ClientSession() as session: + bsblan = BSBLAN(BSBLANConfig(host="example.com"), session=session) + bsblan._supports_full_config = True + bsblan._api_data = { # type: ignore[assignment] + "hot_water": { + "1640": "legionella_function", + "1645": "legionella_function_setpoint", + "1648": "legionella_circulation_temp_diff", + } + } + bsblan._validator._api_validator = APIValidator(bsblan._api_data) + + requests: list[set[str]] = [] + + async def mock_request( + params: dict[str, str] | None = None, + **_kwargs: Any, + ) -> dict[str, Any]: + param_ids = set((params or {})["Parameter"].split(",")) + requests.append(param_ids) + return {param_id: {"value": "1", "unit": ""} for param_id in param_ids} + + bsblan._request = mock_request # type: ignore[method-assign] + + await bsblan._ensure_hot_water_group_validated( + "config", + {"1640", "1645", "1648"}, + include=["legionella_function"], + ) + await bsblan._ensure_hot_water_group_validated( + "config", {"1640", "1645", "1648"} + ) + await bsblan._ensure_hot_water_group_validated( + "config", {"1640", "1645", "1648"} + ) + + assert requests == [{"1640"}, {"1645", "1648"}] + assert set(bsblan._validator._hot_water_param_cache) == { + "1640", + "1645", + "1648", + } + + +@pytest.mark.asyncio +async def test_ensure_hot_water_group_validated_loads_different_include() -> None: + """Test a different include validates its newly requested parameter.""" + async with aiohttp.ClientSession() as session: + bsblan = BSBLAN(BSBLANConfig(host="example.com"), session=session) + bsblan._supports_full_config = True + bsblan._api_data = { # type: ignore[assignment] + "hot_water": { + "1640": "legionella_function", + "1645": "legionella_function_setpoint", + } + } + bsblan._validator._api_validator = APIValidator(bsblan._api_data) + + requests: list[set[str]] = [] + + async def mock_request( + params: dict[str, str] | None = None, + **_kwargs: Any, + ) -> dict[str, Any]: + param_ids = set((params or {})["Parameter"].split(",")) + requests.append(param_ids) + return {param_id: {"value": "1", "unit": ""} for param_id in param_ids} + + bsblan._request = mock_request # type: ignore[method-assign] + + await bsblan._ensure_hot_water_group_validated( + "config", + {"1640", "1645"}, + include=["legionella_function"], + ) + await bsblan._ensure_hot_water_group_validated( + "config", + {"1640", "1645"}, + include=["legionella_function_setpoint"], + ) + + assert requests == [{"1640"}, {"1645"}] + + +@pytest.mark.asyncio +async def test_concurrent_hot_water_includes_recheck_coverage() -> None: + """Test a waiting include request validates its own uncovered parameter.""" + async with aiohttp.ClientSession() as session: + bsblan = BSBLAN(BSBLANConfig(host="example.com"), session=session) + bsblan._supports_full_config = True + bsblan._api_data = { # type: ignore[assignment] + "hot_water": { + "1640": "legionella_function", + "1645": "legionella_function_setpoint", + } + } + bsblan._validator._api_validator = APIValidator(bsblan._api_data) + + requests: list[set[str]] = [] + request_started = asyncio.Event() + + async def slow_request( + params: dict[str, str] | None = None, + **_kwargs: Any, + ) -> dict[str, Any]: + param_ids = set((params or {})["Parameter"].split(",")) + requests.append(param_ids) + request_started.set() + await asyncio.sleep(0.1) + return {param_id: {"value": "1", "unit": ""} for param_id in param_ids} + + bsblan._request = slow_request # type: ignore[method-assign] + + first = asyncio.create_task( + bsblan._ensure_hot_water_group_validated( + "config", + {"1640", "1645"}, + include=["legionella_function"], + ) + ) + await request_started.wait() + second = asyncio.create_task( + bsblan._ensure_hot_water_group_validated( + "config", + {"1640", "1645"}, + include=["legionella_function_setpoint"], + ) + ) + + await asyncio.gather(first, second) + + assert requests == [{"1640"}, {"1645"}] diff --git a/tests/test_include_parameter.py b/tests/test_include_parameter.py index 11078311..d4c028af 100644 --- a/tests/test_include_parameter.py +++ b/tests/test_include_parameter.py @@ -235,8 +235,9 @@ async def test_validate_section_skips_params_not_in_include() -> None: # Validate with include filter - should skip params not in include api_validator.validate_section("heating", request_data, include=["hvac_mode"]) - # Section should be validated - assert api_validator.is_section_validated("heating") + # Only the included parameter should be covered. + assert api_validator.are_parameters_validated("heating", {"700"}) + assert not api_validator.is_section_validated("heating") # hvac_mode (700) should still be in config since it was valid # Other params should NOT be removed since they weren't in include diff --git a/tests/test_reset_validation.py b/tests/test_reset_validation.py index b1ccc16e..29d4ee30 100644 --- a/tests/test_reset_validation.py +++ b/tests/test_reset_validation.py @@ -27,6 +27,18 @@ def test_reset_validation_specific_section() -> None: assert validator.is_section_validated("sensor") is True +def test_reset_validation_clears_parameter_coverage() -> None: + """Test resetting a section clears its recorded parameter coverage.""" + validator = APIValidator({"heating": {"700": "operating_mode"}}) + validator.validate_section("heating", {"700": {"value": "1"}}) + + assert validator.are_parameters_validated("heating", {"700"}) is True + + validator.reset_validation("heating") + + assert validator.are_parameters_validated("heating", {"700"}) is False + + def test_reset_validation_nonexistent_section() -> None: """Test resetting validation for a section that wasn't validated.""" # Create a test API config