Skip to content

feat(core): integrate translator nodes into the graph - #1003

Open
marcpaterno wants to merge 1 commit into
stack/concept-registry/04-conversion-recordsfrom
stack/concept-registry/05-translator-integration
Open

marcpaterno wants to merge 1 commit into
stack/concept-registry/04-conversion-recordsfrom
stack/concept-registry/05-translator-integration

Conversation

@marcpaterno

Copy link
Copy Markdown
Member

Summary

PR 5 of the five-PR concept-registry stack; review against
stack/concept-registry/04-conversion-records.

Add explicit translate registration for one-input/one-output algorithms using
the existing runtime-base and typed-node architecture.

framework_graph / graph_proxy.translate
  -> registration builder -> typed translator_node
  -> runtime declared_translator (consumer + independent producer)
  -> retrieve selected input -> invoke callable -> emit produced result

result metadata
  creator = translator algorithm       suffix = registered output suffix
  index   = input data-cell index      stage  = translator execution stage
  • Constrain translator callables to exactly one input parameter and one output
    object; expose registration through the framework graph and typed graph proxy.
  • Add translator storage/registration, consumer and producer collection,
    predicate-edge wiring, execution counting, and build entries.
  • Emit a new product store containing the produced result, reusing the input
    index and message ID, with translator output specifications and execution stage.
  • Add an end-to-end provider -> translator -> observer regression checking value,
    creator, suffix, layer, distinct input/output stages, index-pointer identity,
    and execution counts.

This does not connect execution to concept conversion records or enforce concept
membership. It does not add discovery, registration-only conversion, module-level
translate exposure, input creator/suffix/stage inheritance, or borrowed-source
lifetime retention. Existing upstream producer/selector behavior is preserved;
Python, FORM, and naming-consumer implementations are unchanged.

Evidence

  • Before: The parent has concept conversion records but no explicitly
    registered executable translator node in the computational graph.
    After: function_registration passes 26 assertions in 2 cases, including
    translation to 1.5, downstream consumption, and all metadata/count checks.
  • Full configured build passed, including Python and FORM targets.
  • Focused graph/catalog/selector/Python/FORM selection: 43/43 CTests passed.
    Configured py:phlex: 11 pytest cases passed, none skipped.
  • Full boundary suite: 119/119 CTests passed. Whole-stack reconciliation subsequently
    reran the final-tip build and all 119 CTests successfully.
  • All 13 changed files passed hooks and whitespace checks. Runtime and typed
    translator implementations match the preserved original source exactly.

The before/after is the feature delta, not a failing-parent test claim. Evidence
is local macOS/AppleClang with Python and FORM ROOT_TTREE; no Linux, sanitizer,
cppyy, coverage-instrumentation, or RNTuple claim. Predicate wiring is inspected,
but no dedicated translator predicate-filter or resource-arbitration regression
is claimed.

Merge Danger

Door: Two-way

No persisted-format migration. Reversion requires removing translator registrations
and dependent workflows; this is the broadest runtime change in the stack.

Blast Radius: Graph

Translators affect product routing, catalog participation, predicate edges, and
execution accounting. Outputs have the translator's identity and execution stage,
not the input's identity/stage. A conversion returning a borrowed view is not made
safe by this node: its new store does not retain the input store for that purpose.


Stack created with GitHub Stacks CLI • Give Feedback 💬

@marcpaterno
marcpaterno added this pull request to stack #1004 October 21, 2026 17:45
@coderabbitai

coderabbitai Bot commented Oct 21, 2026 •

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Next included review available in 59 minutes.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ 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: 4491a372-49ce-46cb-b67f-1c16a4afa23d
📥 Commits

Reviewing files that changed from the base of the PR and between 67942f7 and 1c48cee.

📒 Files selected for processing: 13
  • phlex/core/CMakeLists.txt
  • phlex/core/concepts.hpp
  • phlex/core/declared_translator.cpp
  • phlex/core/declared_translator.hpp
  • phlex/core/framework_graph.cpp
  • phlex/core/framework_graph.hpp
  • phlex/core/glue.hpp
  • phlex/core/graph_proxy.hpp
  • phlex/core/node_catalog.cpp
  • phlex/core/node_catalog.hpp
  • phlex/core/registration_api.hpp
  • phlex/core/translator_node.hpp
  • test/function_registration_test.cpp
  • 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.

@codecov

codecov Bot commented Oct 21, 2026 •

Copy link
Copy Markdown

Codecov Report

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

Files with missing lines Patch % Lines
phlex/core/translator_node.hpp 87.87% 4 Missing ⚠️
@@                               Coverage Diff                                @@
##           stack/concept-registry/04-conversion-records    #1003      +/-   ##
================================================================================
+ Coverage                                         89.45%   89.47%   +0.01%     
================================================================================
  Files                                               179      182       +3     
  Lines                                              6176     6235      +59     
  Branches                                           1031     1033       +2     
================================================================================
+ Hits                                               5525     5579      +54     
- Misses                                              407      411       +4     
- Partials                                            244      245       +1     
Flag Coverage Δ
amd64 89.47% <93.22%> (+0.01%) ⬆️
arm64 89.47% <93.22%> (+0.01%) ⬆️

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

Files with missing lines Coverage Δ
phlex/core/declared_translator.cpp 100.00% <100.00%> (ø)
phlex/core/declared_translator.hpp 100.00% <100.00%> (ø)
phlex/core/framework_graph.cpp 100.00% <100.00%> (ø)
phlex/core/framework_graph.hpp 100.00% <100.00%> (ø)
phlex/core/glue.hpp 100.00% <100.00%> (ø)
phlex/core/graph_proxy.hpp 100.00% <ø> (ø)
phlex/core/node_catalog.cpp 95.83% <100.00%> (+0.37%) ⬆️
phlex/core/node_catalog.hpp 100.00% <100.00%> (ø)
phlex/core/registration_api.hpp 100.00% <ø> (ø)
phlex/core/translator_node.hpp 87.87% <87.87%> (ø)

... and 1 file 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 67942f7...1c48cee. 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 21, 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/translator_node.hpp:38: warning: redundant 'typename'
/__w/phlex/phlex/phlex-src/phlex/core/translator_node.hpp:39: warning: redundant 'typename'

Download clang-tidy artifacts

Register one-input, one-output translators as consumers and independent
producers on the runtime/typed-node architecture.

Preserve translator creator, registered suffix, input data-cell index,
and execution stage, with end-to-end observer regression coverage.
@marcpaterno
marcpaterno force-pushed the stack/concept-registry/05-translator-integration branch from 97aff1d to 1c48cee Compare October 22, 2026 15:46
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