Skip to content

refactor(product): mark_active_param takes only the ref_id - #63

Merged
kewde merged 1 commit into
fix/dynamic-activity-gates-encodingfrom
refactor/mark-active-param-ref-id-only
Sep 15, 2026
Merged

kewde merged 1 commit into
fix/dynamic-activity-gates-encodingfrom
refactor/mark-active-param-ref-id-only

Conversation

@kewde

@kewde kewde commented Sep 13, 2026

Copy link
Copy Markdown
Collaborator

Summary

Follow-up to #62. EvalContext.mark_active_param took both a
ParameterRef id and the Parameter/UnionParameter id it targets,
with the caller (ParameterRefRefNode) responsible for resolving the
latter itself. Since EvalContext already holds the
ApplicationIndexer, and ApplicationIndexer.parameter_refs already
maps ParameterRef.id -> ParameterRef.ref_id (the id it targets), the
resolution belongs in EvalContext itself rather than at every call
site.

  • mark_active_param(ref_id) now takes a single argument and resolves
    parameter_id = self._idx.parameter_refs[ref_id].ref_id internally.
  • EvalContext.idx is now a required, keyword-only argument instead
    of ApplicationIndexer | None with a dead-branch RuntimeError in
    allocate() - every real construction path already supplied a real
    indexer, so the None case only existed in tests.
  • All ~60 test call sites across packages/product/tests/parser_v2/
    now pass idx, via a new shared idx pytest fixture
    (tests/parser_v2/conftest.py) for tests that don't care what's in
    the indexer, or a locally-built one for tests (test_context.py,
    test_module.py) that already had their own.
  • Removed test_allocate_without_indexer_raises, which tested the
    idx=None path that no longer exists.

No behavior change: encode_to_memory() output for the
gira_2gang_button_interface.knxprod fixture is still byte-identical
before and after this change.

Test plan

  • uv run pytest packages/product (536 passed)
  • uv run ruff check / uv run ruff format --check
  • uv run pyright packages/product (same 30 pre-existing errors as base branch, zero new)
  • encode_to_memory() output byte-identical against the real fixture, before and after
  • apps/knx-gui imports and its test suite pass unaffected

🤖 Generated with Claude Code

https://claude.ai/code/session_019yPeudwyfcjBGgTNN3Ad7p

@codecov

codecov Bot commented Sep 13, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 59.47%. Comparing base (22a6b96) to head (dfad269).

Additional details and impacted files
@@                           Coverage Diff                           @@
##           fix/dynamic-activity-gates-encoding      #63      +/-   ##
=======================================================================
- Coverage                                59.48%   59.47%   -0.01%     
=======================================================================
  Files                                      137      137              
  Lines                                    10398    10397       -1     
  Branches                                  1432     1431       -1     
=======================================================================
- Hits                                      6185     6184       -1     
  Misses                                    4081     4081              
  Partials                                   132      132              
Files with missing lines Coverage Δ
.../product/src/xknxmono/product/parser_v2/context.py 100.00% <100.00%> (ø)
...xmono/product/parser_v2/nodes/parameter_ref_ref.py 100.00% <100.00%> (ø)
...es/product/src/xknxmono/product/parser_v2/state.py 100.00% <ø> (ø)
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@kewde
kewde added this pull request to stack #64 September 14, 2026 18:08
stack merge was automatically disabled September 14, 2026 18:13

Pull Request is not mergeable

stack merge was automatically disabled September 14, 2026 19:08

Pull Request is not mergeable

…parameter_id internally

EvalContext already holds the ApplicationIndexer, and parameter_refs
already maps ParameterRef id -> the Parameter/UnionParameter id it
targets, so the caller (ParameterRefRefNode) no longer needs to look
up and pass the target id itself. This also makes idx required on
EvalContext instead of an Optional with a dead fallback branch in
allocate(), since every real construction path already supplies one.

Claude-Session: https://claude.ai/code/session_019yPeudwyfcjBGgTNN3Ad7p
@kewde
kewde force-pushed the refactor/mark-active-param-ref-id-only branch from 2fe3d40 to dfad269 Compare September 14, 2026 19:31
@kewde
kewde merged commit 693ed48 into master Sep 15, 2026
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant