Skip to content

fix: represent absent and empty creator and suffix explicitly - #1009

Draft
marcpaterno wants to merge 2 commits into
stack/concept-registry/05-translator-integrationfrom
represent-creator-and-suffix-state-explicitly
Draft

marcpaterno wants to merge 2 commits into
stack/concept-registry/05-translator-integrationfrom
represent-creator-and-suffix-state-explicitly

Conversation

@marcpaterno

Copy link
Copy Markdown
Member
  • Model an unqualified creator's plugin and an empty suffix as
    explicit empty values instead of invalid empty identifiers.
  • Make a product specification's creator optional; provider
    construction now requires a present creator.
  • Reject empty algorithm names, empty plugin components, and an empty
    configured module_label instead of accepting them silently.
  • Rename the default source creator from "[Source]" to "phlex_source",
    which is a valid name under the new parser.
  • Return borrowed string views from handle creator() and suffix()
    accessors.
  • Add tests for parsing, equality/hashing, exact product lookup,
    provider construction, and handle views.

Stack created with GitHub Stacks CLI • Give Feedback 💬

@marcpaterno
marcpaterno added this pull request to stack #1004 October 23, 2026 22:34
@coderabbitai

coderabbitai Bot commented Oct 23, 2026

Copy link
Copy Markdown
Contributor

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true
  • 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.

@marcpaterno
marcpaterno force-pushed the represent-creator-and-suffix-state-explicitly branch from f3fdc9b to 588cd43 Compare October 28, 2026 14:51
@codecov

codecov Bot commented Oct 28, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 96.55172% with 2 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
phlex/core/producer_catalog.cpp 50.00% 0 Missing and 1 partial ⚠️
plugins/python/src/modulewrap.cpp 75.00% 0 Missing and 1 partial ⚠️
@@                                 Coverage Diff                                 @@
##             stack/concept-registry/05-translator-integration    #1009   +/-   ##
===================================================================================
  Coverage                                                    ?   89.94%           
===================================================================================
  Files                                                       ?      183           
  Lines                                                       ?     6203           
  Branches                                                    ?     1027           
===================================================================================
  Hits                                                        ?     5579           
  Misses                                                      ?      391           
  Partials                                                    ?      233           
Flag Coverage Δ
amd64 89.94% <96.55%> (?)
arm64 89.94% <96.55%> (?)

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

Files with missing lines Coverage Δ
form/form_module.cpp 93.90% <100.00%> (ø)
phlex/core/consumer.cpp 100.00% <ø> (ø)
phlex/core/detail/make_algorithm_name.cpp 100.00% <100.00%> (ø)
phlex/core/producer.hpp 100.00% <ø> (ø)
phlex/core/producer_catalog.hpp 100.00% <ø> (ø)
phlex/core/product_selector.cpp 89.47% <100.00%> (ø)
phlex/core/product_selector.hpp 100.00% <100.00%> (ø)
phlex/core/provider_node.cpp 100.00% <100.00%> (ø)
phlex/core/upstream_predicates.hpp 93.75% <ø> (ø)
phlex/model/algorithm_name.cpp 71.05% <100.00%> (ø)
... and 9 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 b9bc87c...74241c2. 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

github-actions Bot commented Oct 28, 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/phlex/core/product_selector.cpp:41: warning: unchecked access to optional value
/__w/phlex/phlex/phlex-src/test/product_handle_test.cpp:160: warning: the 'empty' method should be used to check for emptiness instead of comparing to an empty object
/__w/phlex/phlex/phlex-src/test/product_selecting_test.cpp:314: warning: comparison between 'signed' and 'unsigned' integers
/__w/phlex/phlex/phlex-src/test/product_selecting_test.cpp:330: warning: comparison between 'signed' and 'unsigned' integers
/__w/phlex/phlex/phlex-src/test/form/form_basics_test.cpp:704: warning: use designated initializer list to initialize 'const value_type' (aka 'const form::experimental::product_with_name')
/__w/phlex/phlex/phlex-src/test/form/form_basics_test.cpp:705: warning: use designated initializer list to initialize 'const value_type' (aka 'const form::experimental::product_with_name')
/__w/phlex/phlex/phlex-src/test/output_products_test.cpp:119: warning: unchecked access to optional value
/__w/phlex/phlex/phlex-src/test/output_products_test.cpp:201: warning: unchecked access to optional value
/__w/phlex/phlex/phlex-src/test/product_selector_test.cpp:91: warning: unchecked access to optional value
/__w/phlex/phlex/phlex-src/test/product_selector_test.cpp:99: warning: unchecked access to optional value
/__w/phlex/phlex/phlex-src/test/product_selector_test.cpp:142: warning: no header providing "std::nullopt" is directly included

Download clang-tidy artifacts

@marcpaterno
marcpaterno force-pushed the represent-creator-and-suffix-state-explicitly branch 2 times, most recently from 30b106b to 5063d91 Compare October 28, 2026 16:26
- Model an unqualified creator's plugin and an empty suffix as
  explicit empty values instead of invalid empty identifiers.
- Make a product specification's creator optional; provider
  construction now requires a present creator.
- Reject empty algorithm names, empty plugin components, and an empty
  configured module_label instead of accepting them silently.
- Rename the default source creator from "[Source]" to "phlex_source",
  which is a valid name under the new parser.
- Return borrowed string views from handle creator() and suffix()
  accessors.
- Add tests for parsing, equality/hashing, exact product lookup,
  provider construction, and handle views.
Represent product suffixes as suffix_t so explicit empty, named, and
omitted selectors are distinct in matching, ordering, and diagnostics.
An omitted suffix matches any product, an empty suffix matches only the
empty product, and a named suffix matches only that name.

- Route the producer catalog, Python wrappers, and FORM labels through
  suffix_t text; the persisted FORM layout is unchanged.
- Treat an omitted creator as unrestricted, and require a present creator
  when a selector names one.
- Require a type for typed product store lookups, so an untyped "hits"
  specification is no longer a wildcard.
- Add tests for selector states, catalog lookup, graph selection, and FORM
  suffix persistence, and update existing tests to use empty_string{}.
@marcpaterno
marcpaterno force-pushed the represent-creator-and-suffix-state-explicitly branch from 5063d91 to 74241c2 Compare October 28, 2026 16:38
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