fix(core): prevent duplicate registration after registrar moves by knoepfel · Pull Request #1038 · Framework-R-D/phlex

knoepfel · 2026-11-03T23:10:40Z

Extend regression coverage for registrar failures, join receiver lookup, and duplicate resource registration.

Aided by GPT-6.1 Sol

Code: Added explicit move construction and move assignment for registrar. They transfer its state and clear the moved-from creator and predicates to prevent duplicate registration during destruction.
Tests: Added coverage for registrar creator failures and moves, multilayer and fold-join receiver routing, and duplicate resource registrations. The resource tests check that a duplicate throws without constructing another resource and that the original remains accessible.
Test status: Not provided.

coderabbitai · 2026-11-03T23:10:58Z

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: 047ef70d-0de2-4b52-ac72-47bab91f2e64

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 registrar now defines explicit move operations. Tests cover registrar lifecycle behavior, join routing and port metadata, and duplicate registration of unlimited and serialized resources.

Changes: Registrar lifecycle

Layer / File(s)	Summary
Registrar moves and lifecycle validation `phlex/core/registrar.hpp`, `test/core_misc_test.cpp`	Move construction and move assignment transfer registrar state and clear the source creator and predicates. Tests cover creator exceptions, output registration, and error reporting.

Join receiver routing

Layer / File(s)	Summary
Join routing and port validation `test/multilayer_join_test.cpp`	Tests cover receiver mappings, selector ordering, invalid lookups, and port metadata for multilayer, same-layer, single-input, and fold joins.

Duplicate resource registration

Layer / File(s)	Summary
Duplicate registration behavior `test/resources_test.cpp`	Tests verify that duplicate unlimited and serialized resource registrations throw without constructing another instance, and that the original resource remains accessible.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Change: Bug fix

Suggested reviewers: greenc-fnal, marcpaterno

Merge Risk: Minimal · previous review 35c43

No merge-blocking issue is established in the supplied changes.

Pre-merge checks: 5

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title names the fix (preventing duplicate registration after registrar moves) and the core area it touches, which matches the registrar.hpp move constructor/assignment changes. The tests added cov…
Docstring Coverage		No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentat…
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: Title check

Explanation

The title names the fix (preventing duplicate registration after registrar moves) and the core area it touches, which matches the registrar.hpp move constructor/assignment changes. The tests added cover more than moves (creator throwing, join receivers, duplicate resource registration), so the title is narrower than the full changeset but still refers to the main change.

Full details: Docstring Coverage

Explanation

No eligible functions found in the changed files. Skipping docstring coverage check. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 0 functions across 4 files.

Finishing Touches

Generate unit tests (beta)

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

Recent review info

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: 0a47f7e6-0226-4eb9-a2dc-28b118f8be30

Commits

Reviewing files that changed from the base of the PR and between ccba655 and 35c433b.

Files selected for processing: 4

phlex/core/registrar.hpp
test/core_misc_test.cpp
test/multilayer_join_test.cpp
test/resources_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: Analyze cpp with CodeQL
GitHub Check: build (gcc, none, amd64, ubuntu-24.04)
GitHub Check: build (gcc, none, arm64, ubuntu-24.04-arm)
GitHub Check: CodeRabbit
GitHub Check: clang-tidy-check
GitHub Check: coverage (arm64)
GitHub Check: coverage (amd64)

Additional context used

Code guidelines: 1

AGENTS.md — auto-discovered

Path-based instructions: 1

Source excerpt: **Authoritative sources**: `.clang-format` and `.clang-tidy` are the ground truth for formatting and naming.

CodeRabbit inference engine (AGENTS.md)

Files:

test/resources_test.cpp
phlex/core/registrar.hpp
test/core_misc_test.cpp
test/multilayer_join_test.cpp

Learnings: 1

Learning: 2026-11-02T15:37:36.740Z

Learnt from: CR
Repo: Framework-R-D/phlex PR: 0
File: AGENTS.md:0-0
Timestamp: 2026-11-02T15:37:36.740Z
Learning: Applies to **/*.{hpp,cpp} : Source excerpt:
# Phlex — Kilo Agent Instructions

## Code Style and Formatting

### C++

- `east-const` style: `int const x` not `const int x`

Applied to files:

test/resources_test.cpp

Additional comments: 11

test/multilayer_join_test.cpp: 7

1-4: LGTM!

15-20: LGTM!

83-88: LGTM!

101-107: LGTM!

133-165: LGTM!

167-174: LGTM!

176-207: LGTM!

test/resources_test.cpp: 2

47-55: LGTM!

313-331: LGTM!

phlex/core/registrar.hpp: 1

83-89: LGTM!

Also applies to: 93-99

test/core_misc_test.cpp: 1

2-2: LGTM!

Also applies to: 11-12, 14-14, 17-17, 104-122, 124-165

codecov · 2026-11-03T23:23:31Z

Codecov Report

✅ All modified and coverable lines are covered by tests.

@@            Coverage Diff             @@
##             main    #1038      +/-   ##
==========================================
- Coverage   90.51%   90.49%   -0.02%     
==========================================
  Files         177      177              
  Lines        5821     5831      +10     
  Branches      916      916              
==========================================
+ Hits         5269     5277       +8     
- Misses        356      358       +2     
  Partials      196      196

Flag	Coverage Δ
amd64	`90.49% <100.00%> (-0.02%)`	⬇️
arm64	`90.49% <100.00%> (-0.02%)`	⬇️

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

Files with missing lines	Coverage Δ
phlex/core/registrar.hpp	`100.00% <100.00%> (ø)`

... and 3 files with indirect coverage changes

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 ccba655...abec092. Read the comment docs.

🚀 New features to boost your workflow:

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

Extend regression coverage for registrar failures, join receiver lookup, and duplicate resource registration. Aided by GPT-6.1 Sol

knoepfel force-pushed the test/extend-coverage branch from 35c433b to 5185fd5 Compare November 4, 2026 14:41

fix(core): prevent duplicate registration after registrar moves

abec092

Extend regression coverage for registrar failures, join receiver lookup, and duplicate resource registration. Aided by GPT-6.1 Sol

knoepfel force-pushed the test/extend-coverage branch from 5185fd5 to abec092 Compare November 4, 2026 16:26

coderabbitai Bot mentioned this pull request Nov 4, 2026

docs: document resources #1025

Open

Navigation Menu

Repository navigation

fix(core): prevent duplicate registration after registrar moves - #1038

fix(core): prevent duplicate registration after registrar moves#1038
knoepfel wants to merge 1 commit into
Framework-R-D:mainfrom
knoepfel:test/extend-coverage

knoepfel commented Nov 3, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Nov 3, 2026 •

edited

Loading

Review skipped

Uh oh!

codecov Bot commented Nov 3, 2026 •

edited

Loading

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

knoepfel commented Nov 3, 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 Nov 3, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review skipped

Uh oh!

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

Uh oh!

Codecov Report

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

knoepfel commented Nov 3, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Nov 3, 2026 •

edited

Loading

codecov Bot commented Nov 3, 2026 •

edited

Loading