You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@hamilton.apache.org by "charitarthchugh (via GitHub)" <gi...@apache.org> on 2026/08/04 22:46:23 UTC

[PR] Show the result builder as a node in the Hamilton UI [hamilton]

charitarthchugh opened a new pull request, #1678:
URL: https://github.com/apache/hamilton/pull/1678

   Implements [#1150](https://github.com/apache/hamilton/issues/1150)
   
   The combined result returned by `execute()` is assembled by a result builder (`DictResult`,
   `PandasDataFrameResult`, ...) in `do_build_result`, after the last node finishes. That runs
   outside the dataflow, so there is no node to attach a data summary to. Every individual output
   is profiled in the UI; the object the caller actually receives is not.
   
   This adds a synthetic node named `_result_builder` to stand in for that object, so it gets the
   same data observability as any other node. Nothing is needed to enable it — attach a
   `HamiltonTracker` as usual.
   
   ## Before / after
   
   Same dataflow, same requested outputs, same result builder, run twice against the same local
   UI. The only difference is which SDK is on the path.
   
   <!-- attach evidence-before-run11.png and evidence-after-run12.png here -->
   
   Before: 4 nodes, the task table ends at `average_squared`. After: 5 nodes, with
   `_result_builder` (`typing.Any`, success) alongside them. Tags, outputs and duration are
   otherwise identical.
   
   | | nodes with recorded runs |
   |---|---|
   | before | `average_squared, input_numbers, squared, sum_squared` |
   | after | `average_squared, input_numbers, _result_builder, squared, sum_squared` |
   
   Opening the node shows the object the caller received:
   
   <!-- attach evidence-after-node-summary.png here -->
   
   ```
   _result_builder — Result summary
   { 2 items
     "sum_squared":     int 55
     "average_squared": int 11
   }
   ```
   
   A `PandasDataFrameResult` gives a full DataFrame profile in the same view, since the built
   result goes through the existing `process_result` path unchanged.
   
   ## Changes
   
   Five commits, one per layer.
   
   **Tracking server.** `NodeTemplate.classifications` is an `ArrayField` constrained by
   `TextChoices`, not a free-form field, so the SDK cannot send a value the server does not know:
   
   ```python
   class Classification(models.TextChoices):
       transform = "transform", _("Transform")
       ...
       placeholder = "placeholder", _("Placeholder")
   ```
   
   - Add `result_builder` to those choices.
   - New postgres migration. `migrations_sqlite/0001_initial.py` is edited in place, matching how
     `0002`'s `unique_together` is already carried there.
   
   **Frontend.** The `Classification` union mirrors that server field, so it has to widen with it.
   
   - Add `result_builder` to the union. No styling, icon or filter — the node renders through the
     existing paths.
   
   **SDK, node template.** Which nodes feed the result varies per run, so a fixed dependency list
   on the template would be wrong for any subset run.
   
   - Declare no dependencies on the template; record the real ones per run.
   - Use `typing.Any` for the output type, since it varies with the result builder. The real type
     shows up in the per-run summary.
   - Make registration opt-in. Registering the node without also emitting a task run renders it as
     never-executed on every run, which is what the legacy `Driver` in that module would do.
   - Fold the node into the DAG hash when registered. `register_dag_template_if_not_exists` matches
     on that hash alone without inspecting the nodes posted with it, so otherwise a template
     registered before this node existed gets reused and runs log against a node it does not have.
   - Skip the node with a warning if the dataflow already has one by that name. `NodeTemplate` is
     unique on `(name, dag_template)`, so registering both would fail the run.
   
   **SDK, task run.** The combined result only exists at `post_graph_execute` time.
   
   - Log the task run there, in both the sync and async trackers.
   - Build the payload once in a shared, I/O-free helper; the trackers differ only in how they
     reach their client.
   - Log and swallow emission failures. This runs immediately before `log_dag_run_end`, so an
     exception escaping here would leave a successful run rendering as still-running.
   
   **Docs.** Add a "The result builder node" subsection to `docs/hamilton-ui/ui.rst`.
   
   ## How I tested this
   
   - `pytest ui/sdk/tests/test_driver.py ui/sdk/tests/test_adapters.py -q` — 22 passed, 3 skipped.
   - `pre-commit run --files <changed files>`
   - Ran the suite at each of the five commits, so the branch bisects.
   - End-to-end against a local UI backend with the migration applied, each result confirmed in
     postgres:
   
   | Case | Result |
   |---|---|
   | `DictResult` | Node renders; summary is the combined dict; deps narrowed to the requested outputs |
   | `PandasDataFrameResult` | Full DataFrame profile |
   | `materialize()` | Deps recorded correctly, materializer id excluded |
   | Failed run | Nothing emitted; node renders not-executed |
   | Name collision | Warning logged, run succeeded, nothing recorded against the user's node |
   
   ## Notes
   
   **Dependency narrowing.** `Driver.materialize` asks `pre_graph_execute` for
   `final_vars + materializer_vars` but hands `post_graph_execute` only the `final_vars` slice, so
   recording the requested list unfiltered would credit the node with materializers it never saw.
   Narrowing is only sound when the result is keyed by node name, so a dataframe or a custom
   builder's own dict keeps the full list rather than be credited with nothing.
   
   **Not every path sees a built result.** Only `Driver.execute()` does. `raw_execute()` and
   `materialize()` never call a result builder, and async drivers build the result after the tracker
   has logged. The node is still emitted in those cases, summarizing the raw dict of outputs. No
   lifecycle hook reports which result builder ran, or whether one ran at all.
   
   **The node is part of the DAG hash**, so the first tracked run after upgrading registers a new
   version of each dataflow. Existing versions and their runs are untouched.
   
   **It renders unconnected in the DAG view.** The template carries no dependencies by design and
   the DAG view builds edges from template dependencies, so the node sits off to one side. Noted in
   the docs.
   
   **The result is profiled twice.** Each output node is already summarized and the combined result
   contains those same objects, so expect roughly twice the profiling time and payload.
   
   **Not refactored:** `_result_attribute`/`_result_attributes` duplicate the inline
   attribute-shaping already in both trackers' `post_node_execute`. Folding those together means
   editing working code outside this change, so I left it. Happy to do it as a follow-up.
   
   cc @skrawcz
   
   ## Checklist
   
   - [x] PR has an informative and human-readable title (this will be pulled into the release notes)
   - [x] Changes are limited to a single goal (no scope creep)
   - [x] Code passed the pre-commit check & code is left cleaner/nicer than when first encountered.
   - [x] Any _change_ in functionality is tested
   - [x] New functions are documented (with a description, list of inputs, and expected output)
   - [x] Placeholder code is flagged / future TODOs are captured in comments
   - [x] Project documentation has been updated if adding/changing functionality.
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@hamilton.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


Re: [PR] Show the result builder as a node in the Hamilton UI [hamilton]

Posted by "skrawcz (via GitHub)" <gi...@apache.org>.
skrawcz commented on code in PR #1678:
URL: https://github.com/apache/hamilton/pull/1678#discussion_r4112661163


##########
ui/sdk/tests/test_adapters.py:
##########
@@ -52,12 +56,236 @@ def test_adapters():
     assert result == {"a": 1, "b": 3, "c": 6}
 
 
-# def test_async():
-#     # TODO: complete Async
-#     kwargs = adapter_kwargs | dict(
-#         dag_name="async_test_dag",
-#     )
-#     [adapters.AsyncHamiltonAdapter(**kwargs)]
+class RecordingClient(test_tracking.MockHamiltonClient):
+    """Mock client that keeps every task update and attribute, not just the latest."""
+
+    def __init__(self, *args, **kwargs):
+        super().__init__(*args, **kwargs)
+        self.all_task_updates = []
+        self.all_attributes = []
+
+    def update_tasks(self, dag_run_id, attributes, task_updates, in_samples=None):
+        self.all_task_updates.extend(task_updates)
+        self.all_attributes.extend(attributes)
+
+
+def _run_and_record(final_vars, inputs, client, *extra_adapters):
+    tracker = adapters.HamiltonTracker(
+        **(
+            adapter_kwargs
+            | dict(dag_name="result_builder_dag", client_factory=lambda *a, **kw: client)
+        )
+    )
+    dr = (
+        driver.Builder()
+        .with_modules(tests.resources.basic_dag_with_config)
+        .with_config({"foo": "baz"})
+        .with_adapters(tracker, *extra_adapters)
+        .build()
+    )
+    return dr.execute(final_vars=final_vars, inputs=inputs)
+
+
+def _emitted_result_builder_tasks(client):
+    return [u for u in client.all_task_updates if u["node_name"] == "_result_builder"]
+
+
+def test_result_builder_task_run_is_emitted():
+    """A successful run emits a _result_builder task whose deps are the requested outputs."""
+    client = RecordingClient()
+    _run_and_record(["a", "b", "c"], {"a": 1}, client)
+
+    emitted = _emitted_result_builder_tasks(client)
+    assert len(emitted) == 1
+    assert emitted[0]["realized_dependencies"] == ["a", "b", "c"]
+    assert emitted[0]["node_template_name"] == "_result_builder"
+    assert emitted[0]["status"] == adapters.Status.SUCCESS
+
+
+def test_no_result_builder_task_run_when_the_name_is_taken():
+    """The template is skipped on a collision, so the task run is skipped with it.
+
+    Emitting anyway would post a task update keyed on the user's own node of that name.
+    """
+    client = RecordingClient()
+    tracker = adapters.HamiltonTracker(
+        **(
+            adapter_kwargs
+            | dict(dag_name="reserved_name_dag", client_factory=lambda *a, **kw: client)
+        )
+    )
+    dr = (
+        driver.Builder()
+        .with_modules(tests.resources.dag_with_reserved_node_name)
+        .with_adapters(tracker)
+        .build()
+    )
+    assert dr.execute(final_vars=["uses_the_reserved_name"], inputs={"_result_builder": 1}) == {
+        "uses_the_reserved_name": 2
+    }
+
+    assert not _emitted_result_builder_tasks(client)
+
+
+def test_result_builder_task_run_carries_a_result_summary():

Review Comment:
   **Nit / suggestion:** The suite covers `DictResult` thoroughly, but the `PandasDataFrameResult` profile—the headline use case in the PR description—is still only verified manually. Could we add a small test asserting that a DataFrame result produces the expected schema/profile attribute? That would lock the marquee path into CI. Non-blocking.



##########
ui/sdk/src/hamilton_sdk/adapters.py:
##########
@@ -58,6 +58,105 @@ def get_node_name(node_: node.Node, task_id: str | None) -> str:
 LONG_SCALE = float(0xFFFFFFFFFFFFFFF)
 
 
+def _result_attribute(node_name: str, name: str, observation: dict) -> dict:

Review Comment:
   **Nit:** As noted in the PR description, these helpers overlap with the inline attribute-shaping in both trackers' `post_node_execute`. This is reasonable to keep scoped out here, but a follow-up that moves those existing paths onto `_result_attribute` / `_result_attributes` would remove the duplication and prevent the payload shapes from drifting. Non-blocking.



##########
docs/hamilton-ui/ui.rst:
##########
@@ -248,6 +248,43 @@ View a history of runs, telemetry on runs/comparison, and data for specific runs
 .. image:: ../_static/run_data.png
     :alt: Run Data
 
+The result builder node
+-----------------------
+
+The combined result your driver returns is assembled by a
+:doc:`result builder </reference/result-builders/index>` once the last node has finished. That
+happens outside the dataflow, so there was no node to attach a data summary to, and what a run
+actually produced did not show up in the UI.
+
+The tracker synthesizes a node named `_result_builder` to stand in for it. You don't write this
+node and it is not part of your dataflow -- the tracker adds it to the DAG it registers, and
+logs a run for it on every successful tracked run, with the same data observability as any
+other node. Nothing is needed to enable it; just attach a `HamiltonTracker` as usual.
+
+Which nodes fed the result varies from run to run, so the node declares no dependencies and
+records the outputs it combined on each run instead. In the DAG view it therefore appears off to
+one side, rather than downstream of the nodes it summarizes.
+
+Note: the node is part of what identifies a DAG version, so the first tracked run after

Review Comment:
   **Nit / release note:** This is documented clearly here. Since upgrading causes the first tracked run to register a new DAG-template version for every dataflow, could we also mention it in the release notes/CHANGELOG? That will keep operators from being surprised when new versions appear after the upgrade. Non-blocking.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@hamilton.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org