Repository navigation
feat(core): integrate translator nodes into the graph - #1003
marcpaterno wants to merge 1 commit into
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the
 |
| Layer / File(s) | Summary |
|---|---|
Translator callable and consumer contract phlex/core/concepts.hpp, phlex/core/declared_translator.hpp, phlex/core/declared_translator.cpp, phlex/core/CMakeLists.txt |
New concepts define the translator callable shape. The declared translator class combines consumer and producer behavior and requires a product count. |
Translator flow-graph execution phlex/core/translator_node.hpp, phlex/core/registration_api.hpp, phlex/core/CMakeLists.txt |
The translator node invokes the callable, wraps its result in a product store, forwards the input message ID, and tracks calls and per-layer product counts. |
Graph registration and catalog integration phlex/core/framework_graph.hpp, phlex/core/graph_proxy.hpp, phlex/core/glue.hpp, phlex/core/framework_graph.cpp, phlex/core/node_catalog.hpp, phlex/core/node_catalog.cpp, test/function_registration_test.cpp |
The graph APIs register translators. Node catalogs include translator consumers and producers, execution counts include translator calls, and predicate filters connect to translators. The test checks translated output and execution counts. |
Priority:
Estimated code review effort: 3 (Moderate) | ~25 minutes
Change: Feature
Sequence Diagram(s)
sequenceDiagram
participant framework_graph
participant glue
participant translator_node
participant translator_callable
participant downstream_observer
framework_graph->>glue: register translator
glue->>translator_node: construct and register node
translator_node->>translator_callable: invoke with input
translator_callable-->>translator_node: return output
translator_node->>downstream_observer: forward output with input message ID
Suggested reviewers: knoepfel, sabasehrish
Merge Risk: 
d9e39
-
The change is mergeable with a known limitation: translators should return owned results, because a result that borrows from its input can become invalid before an observer uses it.
Pre-merge checks: 
-
Check name Status Explanation Resolution Docstring Coverage Docstring coverage is 14.29% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff with cyclomatic complexity of at least 2. Documenta… Write docstrings for the functions missing them to satisfy the coverage threshold. -
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title "feat(core): integrate translator nodes into the graph" matches the main change. The PR adds translator registration through framework_graph and graph_proxy, a new translator_node flow-graph… 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 "feat(core): integrate translator nodes into the graph" matches the main change. The PR adds translator registration through framework_graph and graph_proxy, a new translator_node flow-graph node, a declared_translator class, and node_catalog wiring. The title is short, specific, and follows a conventional-commit style. It does not mention the end-to-end test, but that is expected.
-
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 14.29% which is insufficient. The required threshold is 80.00%. 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 7 functions across 12 files. (1 skipped: 1 unsupported.)
Resolution
Write docstrings for the functions missing them to satisfy the coverage threshold.
-
Finishing Touches: 💡 1
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- 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.
Comment @coderabbitai help to get the list of available commands.
Codecov Report❌ Patch coverage is
@@ Coverage Diff @@
## stack/concept-registry/04-conversion-records #1003 +/- ##
================================================================================
+ Coverage 89.72% 89.77% +0.04%
================================================================================
Files 180 183 +3
Lines 6124 6183 +59
Branches 1014 1016 +2
================================================================================
+ Hits 5495 5551 +56
- Misses 397 401 +4
+ Partials 232 231 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 1 file with indirect coverage changes Continue to review full report in Codecov by Harness.
🚀 New features to boost your workflow:
|
Clang-Tidy Check ResultsClang-tidy found the following issue(s) on lines changed by this pull request: |
97aff1d to
1c48cee
Compare
4110bb2 to
d9e39e0
Compare
d9e39e0 to
371d585
Compare
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.
371d585 to
b9bc87c
Compare
Summary
PR 5 of the five-PR concept-registry stack; review against
stack/concept-registry/04-conversion-records.Add explicit
translateregistration for one-input/one-output algorithms usingthe existing runtime-base and typed-node architecture.
object; expose registration through the framework graph and typed graph proxy.
predicate-edge wiring, execution counting, and build entries.
index and message ID, with translator output specifications and execution stage.
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
translateexposure, input creator/suffix/stage inheritance, or borrowed-sourcelifetime retention. Existing upstream producer/selector behavior is preserved;
Python, FORM, and naming-consumer implementations are unchanged.
Evidence
registered executable translator node in the computational graph.
After:
function_registrationpasses 26 assertions in 2 cases, includingtranslation to
1.5, downstream consumption, and all metadata/count checks.Configured
py:phlex: 11 pytest cases passed, none skipped.reran the final-tip build and all 119 CTests successfully.
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 💬
framework_graph,graph_proxy, andglue. Translator nodes consume one input, produce one output, and connect to product routing and predicate-filter edges.