Skip to content

refactor: reduce template instantiations during registration - #967

Draft
knoepfel wants to merge 15 commits into
refactor/consumer-interfacefrom
refactor/reduce-instantiations
Draft

knoepfel wants to merge 15 commits into
refactor/consumer-interfacefrom
refactor/reduce-instantiations

Conversation

@knoepfel

@knoepfel knoepfel commented Oct 9, 2026 •

Copy link
Copy Markdown
Member

Stack created with GitHub Stacks CLI • Give Feedback 💬

  • Code: Moved registration, product lookup, resource catalog, source, and type-ID implementations out of headers and into compiled source files.
  • Code: Added shared join_support for join repeaters and index ports. Join nodes now cache input receivers for runtime lookup.
  • Code: Canonicalized delegated algorithm types and updated fold, observer, predicate, and transform nodes to use algorithm_type.
  • Build system: Added the new implementation files and installed join_support.hpp.
  • Tests: Added coverage for registrar moves and failures, resource registration, join receiver mapping, and algorithm type canonicalization.

@knoepfel
knoepfel added this pull request to stack #968 October 9, 2026 19:01
@coderabbitai

coderabbitai Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Review in Change Stack →

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration
  • Configuration used: Organization UI
  • Review profile: ASSERTIVE
  • Plan: Advanced
  • Run ID: 05cc548c-5092-47b4-b672-b036ef5dd9e8

📥 Commits

Reviewing files that changed from the base of the PR and between 235f34f and a90ef02.


📒 Files selected for processing (35)
  • phlex/core/CMakeLists.txt
  • phlex/core/detail/join_support.cpp
  • phlex/core/detail/join_support.hpp
  • phlex/core/fold_join_node.hpp
  • phlex/core/fold_node.hpp
  • phlex/core/fwd.hpp
  • phlex/core/glue.cpp
  • phlex/core/glue.hpp
  • phlex/core/input_arguments.cpp
  • phlex/core/input_arguments.hpp
  • phlex/core/message.cpp
  • phlex/core/message.hpp
  • phlex/core/multilayer_join_node.hpp
  • phlex/core/observer_node.hpp
  • phlex/core/predicate_node.hpp
  • phlex/core/product_selector.cpp
  • phlex/core/product_selector.hpp
  • phlex/core/registrar.cpp
  • phlex/core/registrar.hpp
  • phlex/core/resource/catalog.cpp
  • phlex/core/resource/catalog.hpp
  • phlex/core/source.cpp
  • phlex/core/source.hpp
  • phlex/core/transform_node.hpp
  • phlex/metaprogramming/delegate.hpp
  • phlex/model/CMakeLists.txt
  • phlex/model/products.cpp
  • phlex/model/products.hpp
  • phlex/model/type_id.cpp
  • phlex/model/type_id.hpp
  • plugins/python/src/modulewrap.cpp
  • test/core_misc_test.cpp
  • test/multilayer_join_test.cpp
  • test/resources_test.cpp
  • test/transform_node_test.cpp

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.


📜 Recent review details
⏰ Context from checks skipped due to timeout. (7)
  • GitHub Check: determine-languages
  • GitHub Check: build (gcc, none)
  • GitHub Check: header-guards-check
  • GitHub Check: cmake-format-check
  • GitHub Check: coverage
  • GitHub Check: clang-tidy-check
  • GitHub Check: clang-format-check

🧰 Additional context used
📚 Code guidelines (1)
.github/copilot-instructions.md — auto-discovered

📓 Path-based instructions (1)
Source excerpt: Use clang-format tool for all C++ code (VS Code auto-formats on save)

📄 CodeRabbit inference engine (.github/copilot-instructions.md)

Files:

  • phlex/core/fold_node.hpp
  • test/resources_test.cpp
  • phlex/core/message.cpp
  • plugins/python/src/modulewrap.cpp
  • test/transform_node_test.cpp
  • phlex/core/input_arguments.cpp
  • phlex/core/predicate_node.hpp
  • phlex/core/transform_node.hpp
  • phlex/core/observer_node.hpp
  • phlex/core/product_selector.cpp
  • phlex/core/source.hpp
  • phlex/core/fwd.hpp
  • phlex/core/resource/catalog.cpp
  • phlex/core/message.hpp
  • test/core_misc_test.cpp
  • phlex/core/source.cpp
  • phlex/model/products.hpp
  • phlex/core/detail/join_support.hpp
  • phlex/model/type_id.cpp
  • phlex/core/glue.cpp
  • phlex/core/registrar.cpp
  • phlex/model/products.cpp
  • test/multilayer_join_test.cpp
  • phlex/core/detail/join_support.cpp
  • phlex/core/glue.hpp
  • phlex/core/registrar.hpp
  • phlex/core/input_arguments.hpp
  • phlex/core/product_selector.hpp
  • phlex/core/multilayer_join_node.hpp
  • phlex/metaprogramming/delegate.hpp
  • phlex/core/resource/catalog.hpp
  • phlex/core/fold_join_node.hpp
  • phlex/model/type_id.hpp


🪛 Cppcheck (2.22.0)
phlex/core/message.cpp

[style] 44-44: The function 'receiver_for' is never used.

(unusedFunction)


phlex/core/input_arguments.cpp

[style] 17-17: The function 'resolve_product' is never used.

(unusedFunction)


phlex/core/product_selector.cpp

[style] 28-28: The function 'format_as' is never used.

(unusedFunction)


phlex/model/type_id.cpp

[style] 16-16: The function 'hash_value' is never used.

(unusedFunction)


phlex/core/glue.cpp

[style] 14-14: The function 'register_source' is never used.

(unusedFunction)




🔇 Additional comments (33)
plugins/python/src/modulewrap.cpp (1)

10-11: LGTM!

Also applies to: 101-101, 103-103, 105-105


phlex/metaprogramming/delegate.hpp (1)

66-66: LGTM!

Also applies to: 70-70, 72-72, 83-84, 92-92, 97-97


phlex/core/fold_node.hpp (1)

72-72: LGTM!


phlex/core/observer_node.hpp (1)

41-41: LGTM!


phlex/core/predicate_node.hpp (1)

43-43: LGTM!


phlex/core/transform_node.hpp (1)

46-46: LGTM!


test/transform_node_test.cpp (1)

19-20: LGTM!

Also applies to: 155-169


phlex/core/CMakeLists.txt (1)

15-15: LGTM!

Also applies to: 23-23, 32-33, 99-99


phlex/core/detail/join_support.hpp (1)

1-49: LGTM!


phlex/core/detail/join_support.cpp (1)

8-56: LGTM!


phlex/core/fold_join_node.hpp (1)

89-108: LGTM!

Also applies to: 144-164, 176-176


phlex/core/multilayer_join_node.hpp (1)

69-107: LGTM!

Also applies to: 147-151, 164-164


phlex/core/message.hpp (1)

18-18: LGTM!

Also applies to: 106-108


phlex/core/message.cpp (1)

44-51: LGTM!


test/multilayer_join_test.cpp (1)

79-85: LGTM!

Also applies to: 99-105, 131-201


phlex/model/products.cpp (1)

13-13: LGTM!

Also applies to: 16-16, 30-33


phlex/model/products.hpp (1)

78-78: LGTM!

Also applies to: 116-117


phlex/model/type_id.hpp (1)

6-6: LGTM!

Also applies to: 12-14, 17-17, 26-26, 102-102, 303-304


phlex/model/type_id.cpp (1)

1-84: LGTM!


phlex/model/CMakeLists.txt (1)

18-18: LGTM!


phlex/core/fwd.hpp (1)

5-5: LGTM!

Also applies to: 9-15


phlex/core/registrar.hpp (1)

52-52: LGTM!

Also applies to: 56-56, 71-71, 76-76, 81-88, 90-92, 95-97, 101-101, 106-112


phlex/core/registrar.cpp (1)

3-10: LGTM!

Also applies to: 13-14, 16-16, 19-104


test/core_misc_test.cpp (1)

2-2: LGTM!

Also applies to: 11-12, 14-14, 17-17, 103-165


phlex/core/glue.cpp (1)

4-5: LGTM!

Also applies to: 10-11, 14-24


phlex/core/glue.hpp (1)

24-24: LGTM!

Also applies to: 34-37, 209-210


phlex/core/source.hpp (1)

26-26: LGTM!


phlex/core/source.cpp (1)

1-9: LGTM!


phlex/core/resource/catalog.cpp (1)

15-15: 🎯 Functional Correctness

Core target already includes resource/catalog.cpp.

phlex_core lists resource/catalog.cpp in its SOURCE entries, so no CMake change is needed.


phlex/core/product_selector.hpp (1)

14-14: LGTM!

Also applies to: 25-25, 28-28, 32-33, 39-39, 46-46, 49-49, 53-54, 59-59, 62-62


phlex/core/product_selector.cpp (1)

11-12: LGTM!

Also applies to: 16-16, 19-56


phlex/core/input_arguments.hpp (1)

7-7: LGTM!

Also applies to: 9-9, 16-20, 28-28


phlex/core/input_arguments.cpp (1)

1-57: LGTM!





📝 Walkthrough

Walkthrough

The changes update join-node wiring, algorithm type handling, registrar and product lookup implementations, resource catalog operations, model type metadata, and Python stringification. Several implementations move from headers to compiled source files.

Changes

Join wiring

Layer / File(s) Summary
Shared join support
phlex/core/detail/join_support.hpp, phlex/core/detail/join_support.cpp, phlex/core/CMakeLists.txt
Adds shared support for repeater creation and index-port management. The core build compiles the implementation and installs the header.
Join-node wiring and receiver lookup
phlex/core/fold_join_node.hpp, phlex/core/multilayer_join_node.hpp, phlex/core/message.hpp, phlex/core/message.cpp
Join nodes use shared support, cache data receivers, and resolve receiver indices at runtime. message adds a bounds-checked receiver lookup.
Join behavior tests
test/multilayer_join_test.cpp
Tests receiver lookup, index-port identity, same-layer joins, single-input joins, and fold-join selector mapping.

Algorithm type canonicalization

Layer / File(s) Summary
Delegated algorithm type
phlex/metaprogramming/delegate.hpp, phlex/core/fold_node.hpp, phlex/core/observer_node.hpp, phlex/core/predicate_node.hpp, phlex/core/transform_node.hpp, test/transform_node_test.cpp
algorithm_bits now uses one template parameter for the delegated algorithm type. Node aliases and tests use that type.

Node and source registration

Layer / File(s) Summary
Registrar lifecycle and node creation
phlex/core/registrar.hpp, phlex/core/registrar.cpp, phlex/core/fwd.hpp, test/core_misc_test.cpp
Registrar methods move to compiled implementations. The registrar handles moves, creator state, node creation, and duplicate names. Tests cover creator exceptions and moves.
Source registration and default indices
phlex/core/glue.hpp, phlex/core/glue.cpp, phlex/core/source.hpp, phlex/core/source.cpp, phlex/core/CMakeLists.txt
glue::add_source delegates insertion and duplicate-name reporting to a helper. The default source::indices() implementation completes without yielding values.

Product selector lookup

Layer / File(s) Summary
Selector name conversions
phlex/core/product_selector.hpp, phlex/core/product_selector.cpp
creator_name and layer_name become exported classes with out-of-line constructors and conversions. Their implementations validate empty identifiers and define absent-name conversions.
Product specification resolution
phlex/core/input_arguments.hpp, phlex/core/input_arguments.cpp
retriever::retrieve delegates matching and error reporting to resolve_product, which returns the sole matching specification or reports zero or multiple matches.

Resource catalog

Layer / File(s) Summary
Resource registration and lookup
phlex/core/resource/catalog.hpp, phlex/core/resource/catalog.cpp, test/resources_test.cpp
Catalog registration delegates duplicate checks and insertion to implementation methods. Typed lookup uses a type-indexed helper. Tests cover duplicate unlimited and serialized registrations.

Model product and type metadata

Layer / File(s) Summary
Product storage insertion
phlex/model/products.hpp, phlex/model/products.cpp
products::add delegates storage to a helper that appends the specification and moves the product pointer.
Type ID functions and formatting
phlex/model/type_id.hpp, phlex/model/type_id.cpp, phlex/model/CMakeLists.txt
type_id exact-name, hash, and formatting functions move to compiled implementations. The model build includes the new source file.

Python stringification

Layer / File(s) Summary
Const vector stringification
plugins/python/src/modulewrap.cpp
The stringification overloads accept const vectors. Product-selector formatting uses std::views::transform.

Priority: ⚪ Not assessed

Estimated code review effort: 4 (Complex) | ~45 minutes

Change: Refactor


Merge Risk: ⚪ Minimal · up to a90ef

This refactor moves implementations out of line and canonicalizes types, with no behavior change expected. No merge-blocking risk was identified. The review did not build the code.

Pre-merge checks | Passed 4 | Failed 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage Warning Docstring coverage is 8.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 84 functions across 33 files. (2 skipped: … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check Passed The title clearly describes the main objective: reducing template instantiations through out-of-line implementations and explicit registrar instantiations.
Linked Issues check Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check Passed Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 8.33% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 84 functions across 33 files. (2 skipped: 2 unsupported.)



  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 2
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR

🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR


🛠️ Fix failing CI checks 💡
  • Commit to this branch
  • Create a new PR
  • Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@knoepfel
knoepfel force-pushed the refactor/reduce-instantiations branch from 373eb2c to 689b002 Compare October 9, 2026 19:05
@codecov

codecov Bot commented Oct 9, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 87.43455% with 24 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
phlex/core/input_arguments.cpp 26.92% 17 Missing and 2 partials ⚠️
phlex/model/type_id.cpp 94.00% 1 Missing and 2 partials ⚠️
phlex/core/detail/join_support.cpp 95.00% 0 Missing and 1 partial ⚠️
phlex/core/product_selector.cpp 92.85% 1 Missing ⚠️
@@                       Coverage Diff                       @@
##           refactor/consumer-interface     #967      +/-   ##
===============================================================
+ Coverage                        88.11%   88.13%   +0.01%     
===============================================================
  Files                              186      190       +4     
  Lines                             7794     7806      +12     
  Branches                          1023     1024       +1     
===============================================================
+ Hits                              6868     6880      +12     
- Misses                             678      679       +1     
+ Partials                           248      247       -1     
Flag Coverage Δ
scripts 86.35% <ø> (ø)
unittests 88.63% <87.43%> (+0.02%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
phlex/core/fold_join_node.hpp 100.00% <100.00%> (+8.16%) ⬆️
phlex/core/fold_node.hpp 100.00% <ø> (ø)
phlex/core/glue.cpp 100.00% <100.00%> (ø)
phlex/core/glue.hpp 100.00% <100.00%> (ø)
phlex/core/input_arguments.hpp 100.00% <100.00%> (+50.00%) ⬆️
phlex/core/message.cpp 100.00% <100.00%> (+27.27%) ⬆️
phlex/core/message.hpp 100.00% <ø> (ø)
phlex/core/multilayer_join_node.hpp 100.00% <100.00%> (+2.43%) ⬆️
phlex/core/observer_node.hpp 100.00% <ø> (ø)
phlex/core/predicate_node.hpp 100.00% <ø> (ø)
... and 16 more

Continue to review full report in Codecov by Harness.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 235f34f...a90ef02. Read the comment docs.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@knoepfel
knoepfel force-pushed the refactor/reduce-instantiations branch from 689b002 to 75755dc Compare October 9, 2026 19:44
@github-actions

github-actions Bot commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

Clang-Tidy Check Results

Clang-tidy found the following issue(s) on lines changed by this pull request:

/__w/phlex/phlex/phlex-src/test/multilayer_join_test.cpp:85: warning: no header providing "std::size_t" is directly included
/__w/phlex/phlex/phlex-src/phlex/core/detail/join_support.cpp:9: warning: no header providing "std::string" is directly included
/__w/phlex/phlex/phlex-src/phlex/core/detail/join_support.cpp:10: warning: no header providing "std::vector" is directly included
/__w/phlex/phlex/phlex-src/phlex/core/detail/join_support.cpp:16: warning: no header providing "std::nullopt" is directly included
/__w/phlex/phlex/phlex-src/phlex/core/detail/join_support.cpp:30: warning: no header providing "std::optional" is directly included
/__w/phlex/phlex/phlex-src/phlex/core/detail/join_support.cpp:41: warning: no header providing "std::make_unique" is directly included
/__w/phlex/phlex/phlex-src/phlex/core/detail/join_support.cpp:53: warning: no header providing "std::size_t" is directly included

Download clang-tidy artifacts

@knoepfel
knoepfel force-pushed the refactor/reduce-instantiations branch from 75755dc to a90ef02 Compare October 9, 2026 20:11
@knoepfel
knoepfel marked this pull request as draft October 12, 2026 16:51
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