The chat card becomes the full editor
PR #5977 replaces the simplified one-condition approval-rule modal with the same editor the admin page uses, wires the agent to propose-then-confirm, and fixes a graph renderer that was hiding tiered destinations.
A shared EdgeEditorFields component, a validate-only proposeApprovalEdge tool, and a full-edge updateApprovalEdge writer. Plus two unit-test files covering pure logic and summarizer carry-through.
The in-chat ApprovalRulesModal drops its 665-line bespoke single-condition form and renders the shared editor instead. The admin page's EdgeEditorSheet does the same. The graph renders one row per route, not per edge.
The natural-language flow still works: describe a rule, the agent pre-fills the card. The "I don't know how to configure this" escape hatch re-proposes. Nothing writes until the user submits.
1. Why this exists
The admin page already had a full editor (multi-route, nested AND/OR, every operator). The chat surface had its own simplified form that only supported one condition and one destination. Any rule the agent proposed in chat couldn't express what the admin page could — and the two could drift forever.
Even if the agent pre-filled a candidate, the validation was on the writer (addApprovalEdge). A "proposed" edge could pass eyes but fail on submit. There was no validate-only path.
ApprovalGraphView rendered one row per edge and flattened all routes' conditions into it. A tiered rule (>10%→VP, >20%→Deal Desk) collapsed onto a single row with destinations overlapping.
2. The agent flow, step by step
The new path is propose → confirm → write. The agent never writes a rule directly from chat; instead it validates a candidate, the orchestrator opens the editor card pre-filled, and only the user's submit triggers the actual write. Step through:
3. The shared editor: one component, two surfaces
The headline refactor: EdgeEditorFields is extracted to a new file and used from both the admin sheet and the chat card. It's controlled — the parent owns state — so neither surface ships its own persistence or sheet chrome.
ApprovalRulesView rendered the full editor inline (~260 lines), and ApprovalRulesModal was a 665-line bespoke single-condition form with its own operator list, value-editor branching, and "destinations" array that didn't even nest under routes.
- Chat: one condition, one trigger, N destinations (flat).
- Admin: N routes × N blocks × N conditions.
- Grammar mismatch on every submit.
Both surfaces import:
import {
EdgeEditorFields,
isEdgeValid,
type WorkflowEdge,
} from './EdgeEditorFields';
The wrappers contribute only the sheet shell, submit button, and persistence wiring. isEdgeValid mirrors the server's validateEdgeInput, so the Save button enables exactly when the server would accept the edge.
4. Validation: client mirror, server source-of-truth
The server's edge logic is split into a pure module so it can be unit-tested and shared by the new proposeApprovalEdge tool. The client's isEdgeValid intentionally mirrors it.
| Rule | Server (validateEdgeInput) | Client (isEdgeValid) |
|---|---|---|
Trigger required; term→termId, quote→path | ✓ | ✓ |
| At least one route, each with a destination + blocks | ✓ | ✓ |
| Every condition has key + operator | ✓ | ✓ |
exists / not_exists need no value | ✓ | ✓ |
in / not_in require non-empty values | ✓ | ✓ |
between requires finite min AND max (rejects '') | ✓ | ✓ |
Product-trigger edges can't reference quote/opportunity | ✓ | ✓ |
| Term-trigger string values must match flowSpec options | ✓ (IO) | — (server-only) |
| Product-name references valid in catalog | warns (IO) | — (server-only) |
proposeApprovalEdge will always persist cleanly through addApprovalEdge / updateApprovalEdge, because all three call the same validateEdgeInput + validateEdgeIO + normalizeEdge. No more "card closes, then fails in chat."
The empty-string bug that hasFiniteNumber fixes
// Naive — accepts a cleared bound:
Number.isFinite(Number(cond.min)) // Number('') === 0 → true ❌
// New helper:
const hasFiniteNumber = (x) =>
x !== undefined && x !== null && x !== '' && Number.isFinite(Number(x));
Paired on the client: cleared numeric inputs now resolve to undefined instead of snapping to 0. So a user wiping the "min" field doesn't silently create a min: 0 rule.
5. The propose tool and the summarizer channel
The agent needs a way to hand a candidate edge to the orchestrator without writing it. The mechanism is a top-level proposedEdge field on the sub-agent's JSON response — which is only useful if the summarizer doesn't strip it.
Runs the same validateEdgeInput + validateEdgeIO as addApprovalEdge, then returns { success, edge, warnings }. Writes nothing. The orchestrator unwraps edge (the nested value, not the whole envelope) and passes it as the card's prefill.edge.
subAgentSummarizer's KEPT_FIELDS gains one entry:
// Carries the candidate edge from approvalRuleAgent's
// proposeApprovalEdge / getApprovalEdge up to the orchestrator
// so it can pre-fill the approval-rule editor card. Without
// keeping it here, the edge is stripped before the orchestrator
// ever sees it and the card opens empty.
'proposedEdge',
A dedicated unit test pins this behavior: subAgentSummarizer.spec.ts asserts proposedEdge survives.
6. The graph fix: routes, not edges
Before: one row per WorkflowEdge, all of its routes' condition blocks flattened together, destinations overlapping when an edge fanned to multiple groups.
sortedEdges.forEach((edge, i) => {
const allBlocks = edge.routes
.flatMap(r => r.blocks); // ⚠️ flatten
result.push({
id: `cond-${edge.id}`,
data: { blocks: allBlocks, ... },
position: { x: ..., y: i * ROW_HEIGHT },
});
});
Tiered rule ">10→VP, >20→Deal Desk, >30→CFO" rendered as one row with all three conditions jammed together and the three destinations stacked at the same y.
const routeItems = useMemo(() => {
const items = [];
for (const edge of sortedEdges) {
(edge.routes ?? []).forEach((route, ri) => {
items.push({
key: `${edge.id}__${ri}`,
edgeId: edge.id,
blocks: route.blocks,
destination: route.destination,
});
});
}
return items;
}, [sortedEdges]);
Three rows, three condition cells, three arrows landing on three (deduped) group nodes stacked in their own column by chain order. edge.routes ?? [] guards malformed specs.
7. Files touched
8. UI polish in the condition row
ConditionBlockEditor's row layout changes from one wide 5-column grid to two stacked rows inside a bordered card.
grid-cols-[96px_minmax(180px,2.2fr)_76px_minmax(0,1fr)_28px]
The value cell was the narrowest column. Select-option labels got truncated mid-word.
Row 1: [entity 110px | attribute 1fr]
Row 2: [operator 150px | value 1fr | × 28px]
The value cell is now full-width inside row 2. Long enum labels render in full. The whole row is wrapped in a subtle bordered card so multi-condition blocks read as discrete units.
9. What this doesn't change
- The engine's edge evaluation logic —
validateTermTriggerValuesAgainstFlow, the conflict detector, and the runtime predicate map are untouched. updateEdgeRoutesandupdateEdgeMetadatastill exist. They're now narrow tools for partial tweaks; full-edge card edits go through the newupdateApprovalEdge.- The approval-group management tools (
listApprovalGroups,deleteApprovalGroup) are unchanged. - The "Quick Add" library-pick form on the admin page is removed — that flow is collapsed into the unified editor sheet. The
onAskAgentprop and its escape hatch back to chat are gone with it. - No migrations.
workflowSpecshape is unchanged.
10. Risks & things to look at in review
isEdgeValid (client) and validateEdgeInput (server) are deliberately kept in lockstep but live in different files. If a future PR adds an operator or a rule on one side, the other will silently disagree. Worth a comment cross-reference, or eventually shared via the @dealops/types package.
proposedEdge channel is convention, not type-checked. The summarizer keeps the field; the agent's prompt instructs it to set proposedEdge to the nested edge (not the whole { success, edge, warnings } envelope). If the model wraps the envelope by mistake, the card opens blank. Two unit tests pin the summarizer side, but the model-side contract is prose in approvalRuleAgent.ts.
workflowSpec edges and remain valid.
Open questions
- Should the "help" hatch (
[Modal:approval_rules help]) get an integration test? The orchestrator prompt describes the re-propose loop but nothing pins it. - The
QUOTE_METRIC_OPTIONSlist is duplicated comment-wise as "mirrorsapprovalRuleAgent.tsfooter-path." That's still a manual sync point — worth extracting to@dealops/types?