[Detail Bug] GUI: Nested submodule parameters cannot be edited due to duplicated qualified ref IDs
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 4
- Forks
- 0
- Avg merge
- 15h 38m
- Merged PRs (30d)
- 37
Description
Detail Bug Report
Introduced in 67c5f5469b469965161345313a1ff178558685c5 by @kewde on Jun 15, 2026
Summary
- Context:
packages/product/src/xknxmono/product/parser_v2/state.pybuilds and queries a tree ofParameterState(GlobalState→ModuleState→ nestedModuleStatechildren) used byparser_v2to expose a dynamic UI (DynamicUI), resolve user parameter edits (set_parameter_ref), and gate encoding. - Bug:
ModuleState._qualifymis-qualifies def-relative parameter ref IDs when the surrounding scope is a nested submodule, producing a qualified ID with a duplicated_SM-…segment. - Actual vs. expected: Actual:
qualify(local)returns…_MI-1_SM-1_M-…_MI-1_SM-1_P-…(duplicated_SM-1), andset_parameter_refwrites the new value under a stale local key (…_SM-1_SM-1_P-…) that the read path never uses. Expected:qualify(local)returns…_MI-1_SM-1_M-…_MI-1_P-…(single_SM-1), andset_parameter_refupdates the canonical def-relative key so reads immediately reflect the edit. - Impact: parameter refs on nested-submodule parameters are silently un-editable through the GUI; edits are stored under an orphan/stale key and never take effect in UI re-rendering or calculations.
Code with Bug
# packages/product/src/xknxmono/product/parser_v2/state.py
class ModuleState(ParameterState):
def _qualify(self, ref_id: str) -> str:
i, n = 0, min(len(self.module_instance_id), len(ref_id))
while i < n and self.module_instance_id[i] == ref_id[i]:
i += 1
return self.module_instance_id + ref_id[i - 1 :] # <-- BUG 🔴 LCP slice re-emits nested `_SM-…` segment, duplicating it
Explanation
_qualify attempts to build a fully-qualified UI ref ID by taking a longest-common-prefix between module_instance_id and a def-relative ref_id, then appending ref_id[i-1:]. This happens to work for top-level modules, where the common prefix ends exactly at the module def ID boundary.
For nested submodules, the common prefix stops earlier (at the parent ModuleDef boundary where instance tokens diverge from submodule-def tokens), so ref_id[i-1:] includes the submodule token that is already present in module_instance_id. The qualified ID therefore contains a duplicated _SM-… segment.
The write path for GUI edits (DynamicUI.set_parameter_ref → state.set_instance_ref) uses find_scope_for_qualified to invert the qualified ID back into a local key. That inversion is not symmetric with _qualify for nested submodules: it selects the correct child scope but reconstructs a non-canonical local key (…_SM-1_SM-1_P-…). The new value is written under that stale key, while all reads (child.get(canonical_local)) continue to use the canonical def-relative key (…_SM-1_P-…), so the UI immediately re-renders the old value and calculations see the old value as well.
Recommended Fix
Replace the LCP character-scan with a prefix-based slice that mirrors find_scope_for_qualified:
def _qualify(self, ref_id: str) -> str:
if self.ref_id is None or not ref_id.startswith(self.ref_id):
return ref_id
return self.module_instance_id + ref_id[len(self.ref_id):] # <-- FIX 🟢 append only the def-relative suffix
Add a regression test for nested submodules asserting:
child.qualify(local)contains only a single_SM-…segmentchild.find_scope_for_qualified(child.qualify(local)) == (child, local)(round-trip identity)
History
This bug was introduced in commit 67c5f54. The refactor added ModuleState._qualify using a longest-common-prefix scan plus ref_id[i - 1:], which works for top-level modules but duplicates the _SM-… token for nested submodules. A later commit, 7bb6a6a, introduced the asymmetric inverse find_scope_for_qualified and the set_parameter_ref/set_instance_ref routing path that flows through it, making the duplicate-segment issue manifest as a silent edit failure for nested-submodule parameters.
Contributor guide
No contributing guide indexed for this repository
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in packages/product/src/xknxmono/product/parser_v2/state.py at ModuleState._qualify, then trace DynamicUI.set_parameter_ref and find_scope_for_qualified. Add the nested-submodule regression coverage described in the issue, including the qualification and round-trip assertions; done means edits resolve to the canonical key and reads reflect the new value.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- frontend, testing
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 76/100