Write API Phase 1 — 6: Pipeline façade — edge endpoints - #4331
Draft
SmittieC wants to merge 7 commits into
Draft
Conversation
`unwired_handles` knew inline that Start is the one node type with no input handle. The pipeline edge endpoints need the same fact on the write side, so it moves into `input_handles()` beside `output_handles()` rather than being restated there. A list, not a flag, so a caller reads both sides the same way. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
POST /chatbots/{id}/pipeline/edges/ and DELETE .../edges/{edge_id}/, the edge
half of the pipeline façade (#4141). Both run the same locked read-modify-write
the node endpoints do and answer with the same envelope: what was written, then
the state of the pipeline it was written into.
What a wire refuses, and what it merely reports, follows the node endpoints'
rule. A request naming something the pipeline does not have is refused: an
endpoint that is not a node here, a handle the source does not offer, a pair
already wired. A graph that is only *wrong* -- a cycle, an End node nothing
reaches -- persists and comes back in `pipeline_errors`, because which edge of a
cycle is the wrong one is the caller's call, not the server's.
Handles are named only when there is a choice to make. A source offering one
output handle needs none; a router offers one per branch, so leaving it out is
refused with the branches listed. `target_handle` is never required. A null
handle reads as an omitted one, so an edge read back from /inspect/ can be sent
straight back.
Edge ids are the server's, built with react-flow's own `getEdgeId` formula over
the wiring so an API-wired edge reads like a hand-wired one. That form is derived
rather than random, so it can collide: a keyword edit leaves an edge holding the
id of a wiring it no longer has, and the patch engine treats an add whose id
already exists as a no-op. So a taken id gets a suffix, and the response reads
the edge back out of the saved graph rather than reporting the planned one.
Re-wiring a pair already wired that way is refused rather than stored twice,
which is what makes the most retry-prone write in the API safe to retry: a repeat
leaves the graph exactly as the first call left it, and the refusal names the
edge that already wires the pair.
Nothing moves on the canvas: a node keeps the position it was parked at, so
wiring cannot shuffle a layout someone arranged in the pipeline builder.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`target_handle` was documented as "currently always null", which the edge endpoints falsify: every edge they create stores `input`. It was already loose -- the default pipeline `create_pipeline_with_nodes` builds stores `input` too -- and it ships in api-schemas/v2.yml, so it reaches an API consumer as fact. Null is specifically what an edge the pipeline builder drew carries, because the builder renders no id on its target handles. Says that instead. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Three facts about a graph were each written down in several places, and the edge endpoints added a copy of all three. `FlowEdge` gains `source_handle_name` / `target_handle_name` / `wiring`. "An absent handle means the node's only one" was spelled out at four sites; the fields whose defaults declare that rule are where it belongs. `graph.Edge` keeps its own copy -- a separate model, with no `targetHandle` field at all. `react_flow_edge_id` moves to `flow.py`, beside `react_flow_node_type`. That module already owns the react-flow conventions we borrow, so the API app no longer needs to know how the editor shapes an edge id. Kept a pure string function: `flow.py` is imported by a data migration and must stay off the node-class registry. `why_no_output_handles` moves next to `output_handles`, which collapses three different situations into an empty list. Re-deriving which one applied in the API app meant two branch structures that could drift into disagreeing, and a fourth handle-less case would silently have been blamed on the End node. It answers with an enum now, and the API app renders wording from a mapping keyed on it, so a new case raises rather than inheriting the End node's answer. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The edge endpoints arrived as a second view class repeating the first's `permission_classes`, `required_scopes` and `queryset` verbatim. An auth surface in two copies is one a change can be applied to half of, so a `PipelineFacadeView` base carries them now -- beside its subclasses, as `DiscoveryView` already does for the discovery endpoints. The response envelope goes with it. Each view rebuilt the same skeleton and hardcoded its own `"node"` / `"edge"` key, which the response serializer's `written_field` already declares: two sources of truth for one fact, where drift would advertise a key the body does not carry. The base reads the key off the serializer and calls one `_written` hook per view. The hooks stay separate -- they read different stores (rows vs the JSON blob, ADR-0049) and each guards a different failure, so only the skeleton was ever shared. `PipelineEdit` loses its second field. It carried `node_id` *and* an `edge`, of which only the id was ever read -- the response is rebuilt from storage anyway -- so "one or the other, never both" needed a `__post_init__` to police it. One `written_id` makes that unrepresentable, and narrows `respond` back to the signature it had before the edge endpoints. Also here, because it is the same function's business: `_persist` no longer reconciles node rows for a diff that cannot touch one, which only the edge endpoints produce. `update_nodes_from_data` wrote nothing for such a diff, but `clear_node_caches` then threw away the `node_set` prefetch the locked read had just paid for, so the build state re-read every row. Takes a wire from 17 statements under the row lock to 10, pinned by `WIRE_QUERIES_UNDER_THE_LOCK`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
`conftest` grew `boundary_node`, `wire`, `add_router_node` and `inspect_url` while the edge tests were written, but the node tests kept the hand-rolled forms they replaced -- seven copies of the Start/End lookup, three of "create a node through the API", two of "wire through the API". A helper that is canonical for half a package and absent from the other half is one the next person copies from whichever file they happened to read first. Also: merged two tests asserting one guarantee about node rows, dropped one subsumed by the parametrised test below it and one unused `llm` fixture, named the `spliced` fixture's three ids so eight tests stop opening with positional bookkeeping, and replaced `_call`'s verb chain with a table -- a `KeyError` names the typo that the chain's fall-through would silently have mis-exercised. Dropped the `server_assigned_keys` meta-test: the bare annotation already raises on a serializer that forgets to declare its keys, and the test only ever saw direct subclasses. `WIRE_QUERIES_UNDER_THE_LOCK` records the two rules behind its number, since an absolute pin can break for unrelated reasons: restoring the node reconcile costs a flat 7 statements at any graph size, and each extra node on the fixture graph costs 2. So a failure at 17 is the revert and a failure at 12 is a changed fixture, without the reader having to measure to find out which. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
SmittieC
force-pushed
the
cs/edge_api
branch
from
September 1, 2026 14:25
787e7d2 to
b638f18
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #4141. Part of the Phase 1 write API (#4135), stacked on the node endpoints (#4140) — base branch is
cs/pipeline_facade_redone, notmain.Product Description
An agent building a chatbot's pipeline over the API can now connect its nodes, and disconnect them:
sourceandtargetare usually the whole body. A handle left out means the only one the node has, so it is named only when there is a choice to make — which today is a router, whose handles are its branches. Both nodes keep the position they were parked at; nothing is moved on the canvas (W11 is Phase 2).A wire the server can act on always lands, even when it leaves the pipeline unbuildable — an agent is mid-build, and
pipeline_errorsis how it learns what is still wrong. A wire naming something the pipeline does not have is refused instead.Technical Description
Both endpoints run the same locked read-modify-write the node endpoints do, and answer with the same envelope: the resource written, then the state of the pipeline it was written into.
Refused (400), keyed by the field at fault — an endpoint that is not a node in this pipeline; a
source_handlethe source does not offer, or a missing one where it offers a choice; atarget_handlethat is not the target's input; asourcewith no output handles at all; a client-suppliedid; an unrecognised key; a duplicate wire.Reported, not refused — a cycle (
pipeline_errors.pipeline), an End node nothing reaches (pipeline_errors.node[<end>].root), a node whose own params don't validate yet. Which edge of a cycle is the wrong one is the caller's call.Three things worth a reviewer's attention:
getEdgeIdformula over the wiring, so an API-wired edge reads like a hand-wired one. Being derived rather than random, it can collide: a router keyword edit leaves an edge holding the id of a wiring it no longer has (graph_editor._rewired_edges), andpatching._apply_edge_difftreats an add whose id already exists as a no-op — which would answer 201 having stored nothing. So a taken id gets a suffix, and_write_responsereads the edge back out of the saved graph rather than reporting the planned one. Not byte-identical to a builder-drawn id: the builder renders no ids on its target handles, so its edges stop at the target's node id where these end ininput.targetHandlewhere the server storesinput.source_handleis optional, filled in when the source offers exactly one handle. The ticket only stated that oftarget_handle; extending it is strictly more permissive, symmetric, and saves an agent needing to know thatoutputis the magic name. Trade-off worth recording: omitting it binds the caller to "whatever the single handle is", so a node type that later gains a second output turns previously-valid calls into 400s. Equally true oftarget_handle.One deliberate departure from the ticket's shorthand: an unreachable End node is reported under
pipeline_errors.node[<end>].root, notpipeline_errors.pipeline.PipelineBuildErrorcarries the End node's id andexceptions.error_reportattributes any such error to that node — and that bucketing is already what the shipped/inspect/endpoint publishes, so changing it would break a read contract. Cycles do land inpipeline_errors.pipeline. Both endpoint descriptions now spell out the asymmetry.Commits are split by type: a preparatory
refactor:moving "which input handles does this node type have" beside its output twin so both sides read it the same way, thefeat:itself, and adocs:fix to an/inspect/help text this change falsified.Migrations
No migrations. Stored shape is unchanged — edges keep living in
Pipeline.data(ADR-0049) in the shape the pipeline builder already writes, andedit_revisionis bumped on every write so an open builder session sees a conflict rather than overwriting.Demo
Driven end-to-end against a live server on a dev DB: built a router and an LLM node, wired four edges, exercised every refusal, deleted by the id
/inspect/emitted, retried the delete (404), closed a cycle (201 + reported). The pipeline builder's own data endpoint hands back the API-wired edges in exactly the shape it writes its own.Docs and Changelog
The OpenAPI schema is the documentation for this surface and is regenerated in-tree (
api-schemas/v2.yml), including per-shape request examples and the exact duplicate-wire message so a client can anchor on it.Operator Impact
Purely additive API surface: no migrations, no new or renamed settings or env vars, no change in deployment shape, nothing deprecated or removed. Operators learn about it through the user-facing changelog.
Cleanup pass
Four review passes (reuse / simplification / efficiency / altitude) ran over the feature after it was written, and their findings landed as three commits on top. Worth a reviewer's attention:
FlowEdgegainedsource_handle_name/target_handle_name/wiring. "An absent handle means the node's only one" had four copies; the fields whose defaults declare that rule now own it.graph.Edgekeeps its own — a separate model, with notargetHandlefield at all.why_no_output_handles+ aNoOutputHandlesenum moved besideoutput_handles, which is what collapses three situations into an empty list. Deriving which one applied in the API app meant two branch structures that could drift, and a fourth handle-less case would have been blamed on the End node. The API app now renders wording from a mapping keyed on the enum, so a new case raises.PipelineFacadeViewbase holds the permission trio and the response envelope. The gates were declared twice, which is the copy-paste worth removing first. The envelope's key now comes from the response serializer'swritten_fieldrather than a literal repeated per view.PipelineEditcollapsed to onewritten_id, which makes "a node or an edge, never both" unrepresentable rather than policed by a__post_init__._persistno longer reconciles node rows for a diff that cannot touch one.update_nodes_from_datawrote nothing for an edge-only diff, butclear_node_cachesthen discarded thenode_setprefetch the locked read had paid for, so the build state re-read every row. Takes a wire from 17 statements under the row lock to 10, pinned byWIRE_QUERIES_UNDER_THE_LOCK— whose comment records both diagnostic rules, so a failure at 17 identifies itself as a revert and one at 12 as a changed fixture.Each of the three commits was verified to run on its own in a throwaway worktree (221 / 220 / 219 tests), after an earlier split of mine put a call site in one commit and the field it needed in the next.
Follow-ups found on the way, deliberately not in this PR
apps/api/schema.py:243splatssettings.OAUTH2_PROVIDER["SCOPES"], whichconfig/settings.py:1120mutates in place whenOIDC_RSA_PRIVATE_KEYis set — so a developer's local signing key changes a committed artifact and red-teststest_schema_is_up_to_date_and_validon a file they never touched. Contradicts the contract stated atschema.py:18-21;schema.py:275already does it correctly by naming a single scope.inspect/nodes.py::graph_digestemitsourceHandle or "output"/targetHandle or "input"would make the read and write edge shapes one non-nullable component. Not an invention —build_state.unwired_handlesandgraph.Edge.is_conditionalalready read stored edges that way — but it flips a shipped read contract.pipeline_build_stateis an N+1 in node count — 2 queries per LLM-backed node, 4 per router, becauseapps/pipelines/nodes/mixins.py:103,120each construct a freshORMRepository()so the@instance_cacheonget_llm_provider_modelnever hits. Pre-existing and paid by/inspect/too, but these endpoints double the number of write paths paying it, all inside the row lock. The router doubling is a second instance of the same shape:Pipeline._node_validation_errorsandbuild_state._router_output_mapeach validate the same node independently.UserFactoryemail collisions under--reuse-db.apps/utils/factories/user.py:18draws from a per-processfake.unique, so a fresh process can redraw an email a previous run persisted; surfaces as a randomUniqueViolationin an unrelated test.Considered and rejected
Moving the handles trio (
input_handles/output_handles/node_output_handles, plus the newNoOutputHandlesenum) out ofbuild_state.pyinto anapps/pipelines/handles.py. Only the filename argues for it. The enum isoutput_handles' branch structure read a second way — that is why they sit adjacent and why the wording is a total mapping keyed on the enum — so a module boundary between them would reintroduce exactly the drift the pairing prevents. Ifbuild_state.pyever grows an unrelated second concern, the whole trio moves together; tidiness alone is not the trigger.🤖 Generated with Claude Code