refactor: reduce template instantiations during registration by knoepfel · Pull Request #967 · Framework-R-D/phlex

knoepfel · 2026-10-09T19:01:43Z

_{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.

coderabbitai · 2026-10-09T19:01:56Z

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

Run configuration

Source mode: Auto
Repository: Unavailable
Branch: Unavailable
File: No configuration file from this repository applied
Commit: Unavailable
Layers: Organization UI
Review profile: ASSERTIVE
Plan: Advanced
Run ID: e5b9160f-5493-4e40-8154-5fb831005365

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

Trigger review

No actionable comments were generated in the recent review.

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`.

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

Change: Refactor

Merge Risk: Minimal · previous review 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 | 4 | 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage		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		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly describes the main objective: reducing template instantiations through out-of-line implementations and explicit registrar instantiations.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		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.)

Finishing Touches

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

Tip

🎯 Customize your risk assessments

Guide how CodeRabbit assesses merge risk and security architecture risk. Highlight sensitive changes and explain your security boundaries using Merge Risk Instructions and Security Risk Instructions in your review settings. Security architecture risk assessments require the Advanced plan or above.

✅ Automatic approvals, with your rules

Let CodeRabbit approve pull requests once required threads are resolved, pre-merge checks pass, and your risk limits are met. Enable Automatic approvals in review settings, with overrides for teams or file paths.

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

CodeRabbit Verify

Run CodeRabbit Verify to test the changes in this pull request.

ℹ️ 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!

codecov · 2026-10-09T19:12:26Z

Codecov Report

❌ Patch coverage is 97.53086% with 4 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
phlex/model/type_id.cpp	80.95%	4 Missing ⚠️

@@            Coverage Diff             @@
##             main     #967      +/-   ##
==========================================
+ Coverage   89.67%   89.79%   +0.12%     
==========================================
  Files         176      180       +4     
  Lines        6031     6037       +6     
  Branches     1000     1003       +3     
==========================================
+ Hits         5408     5421      +13     
+ Misses        396      391       -5     
+ Partials      227      225       -2

Flag	Coverage Δ
amd64	`89.79% <97.53%> (+0.12%)`	⬆️
arm64	`89.79% <97.53%> (+0.12%)`	⬆️

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

Files with missing lines	Coverage Δ
phlex/core/detail/join_support.cpp	`100.00% <100.00%> (ø)`
phlex/core/fold_join_node.hpp	`100.00% <100.00%> (+6.38%)`	⬆️
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.cpp	`100.00% <100.00%> (ø)`
phlex/core/input_arguments.hpp	`100.00% <100.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%)`	⬆️
... 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 cea210a...2986c7e. Read the comment docs.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

github-actions · 2026-10-09T19:53:54Z

Clang-Tidy Check Results

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

/__w/phlex/phlex/phlex-src/phlex/model/type_id.cpp:45: warning: do not use array subscript when the index is not an integer constant expression
/__w/phlex/phlex/phlex-src/test/multilayer_join_test.cpp:179: warning: variable 'support' of type 'internal::join_support' can be declared 'const'

Download clang-tidy artifacts

-    // in the data hierarchy. Until the router exposes a depth comparison, we pick
-    // layers_[0]; this is only correct when the first input is at the deepest layer.
-    std::vector<named_index_port> index_ports()
+    // FIXME: For multi-input folds, the "most-derived" input layer is the deepest of


Aided by GPT-6.1 Sol

Replace the builtin type formatting switch with a lookup table to eliminate an unreachable branch. Aided by GPT-6.1 Sol

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

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

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

knoepfel force-pushed the refactor/reduce-instantiations branch from 75755dc to a90ef02 Compare October 9, 2026 20:11

knoepfel marked this pull request as draft October 12, 2026 16:51

knoepfel force-pushed the refactor/reduce-instantiations branch from a90ef02 to d519186 Compare November 3, 2026 19:31

github-advanced-security Bot found potential problems Nov 3, 2026

View reviewed changes

Base automatically changed from refactor/consumer-interface to main November 3, 2026 19:59

greenc-FNAL force-pushed the refactor/reduce-instantiations branch from d519186 to c0906e1 Compare November 3, 2026 20:00

knoepfel added 8 commits November 3, 2026 14:00

refactor: move implementation to .cpp

a145716

refactor(registration): canonicalize algorithm types

5974459

refactor(model): define erased product insertion out of line

c605965

refactor(core): move selector wrapper operations out of line

3e5d926

refactor(core): move resource catalog operations out of line

6380b9c

refactor(core): move source registration and indices out of line

876e62a

refactor(model): move runtime type metadata out of line

8f26529

refactor(core): explicitly instantiate registrars out of line

d9eaf9d

knoepfel force-pushed the refactor/reduce-instantiations branch from c0906e1 to 51f5534 Compare November 3, 2026 20:01

knoepfel added 2 commits November 3, 2026 14:29

refactor(core): extract shared join support and cache receivers

91c5cc8

Aided by GPT-6.1 Sol

test(core): cover product resolution and join edge cases

2986c7e

Replace the builtin type formatting switch with a lookup table to eliminate an unreachable branch. Aided by GPT-6.1 Sol

knoepfel force-pushed the refactor/reduce-instantiations branch from 51f5534 to 2986c7e Compare November 3, 2026 20:32

Navigation Menu

Repository navigation

refactor: reduce template instantiations during registration - #967

refactor: reduce template instantiations during registration#967
knoepfel wants to merge 10 commits into
mainfrom
refactor/reduce-instantiations

knoepfel commented Oct 9, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Oct 9, 2026 •

edited

Loading

Review skipped

❌ Failed checks (1 warning)

Uh oh!

codecov Bot commented Oct 9, 2026 •

edited

Loading

Uh oh!

github-actions Bot commented Oct 9, 2026 •

edited

Loading

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

knoepfel commented Oct 9, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot commented Oct 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

❌ Failed checks (1 warning)

CodeRabbit Verify

Uh oh!

codecov Bot commented Oct 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

github-actions Bot commented Oct 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Clang-Tidy Check Results

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

knoepfel commented Oct 9, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Oct 9, 2026 •

edited

Loading

codecov Bot commented Oct 9, 2026 •

edited

Loading

github-actions Bot commented Oct 9, 2026 •

edited

Loading