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.

PRdealops#5977 Author@yijunz166 Branchyijunz/v3-approval-rule-editor Files14 +/−+1272 / −1328 AreaDealops 3 · Config Agent

What it adds

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.

What it changes

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.

What it preserves

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.

Orchestrator
approvalRule sub-agent
Editor card (user)
workflowSpec (draft)
Shared editor / logic

1. Why this exists

Problem 1 — Two editors, two grammars

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.

Problem 2 — Propose-vs-persist drift risk

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.

Problem 3 — Graph hides tiers

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.

Before — divergent

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.
After — shared

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.

RuleServer (validateEdgeInput)Client (isEdgeValid)
Trigger required; termtermId, quotepath
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 catalogwarns (IO)— (server-only)
Key invariant: a candidate from 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.

New tool — proposeApprovalEdge

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.

Summarizer carry-through

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.

Before
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.

After
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

apps/client/src/dashboard/V3Admin/EdgeEditorFields.tsxnew+281
apps/client/src/dashboard/V3Admin/ConfigAgent/modals/ApprovalRulesModal.tsxmod+109 −665
apps/client/src/dashboard/V3Admin/ApprovalRulesView.tsxmod+10 −260
apps/client/src/dashboard/V3Admin/ApprovalGraphView.tsxmod+63 −68
apps/client/src/dashboard/V3Admin/ConditionBlockEditor.tsxmod+119 −109
apps/client/src/dashboard/V3Admin/index.tsxmod−4
apps/server/src/dealops3/configAgent/tools/approvalRuleLogic.tsnew+191
apps/server/src/dealops3/configAgent/tools/approvalRuleTools.tsmod+220 −217
apps/server/src/dealops3/configAgent/tools/modalTools.tsmod+2 −2
apps/server/src/dealops3/configAgent/agents/orchestrator.tsmod+8 −1
apps/server/src/dealops3/configAgent/agents/approvalRuleAgent.tsmod+7 −2
apps/server/src/dealops3/configAgent/subAgentSummarizer.tsmod+5
apps/server/src/dealops3/configAgent/__tests__/approvalRuleLogic.spec.tsnew+205
apps/server/src/dealops3/configAgent/__tests__/subAgentSummarizer.spec.tsnew+52

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.

Before
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.

After
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

10. Risks & things to look at in review

Client/server validation drift. 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.
The 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.
Rollback is clean. No schema changes; reverting the PR removes the new tools and restores the old modal. Drafted rules created via the new path are normal workflowSpec edges and remain valid.

Open questions