Skip to content

Commit 8cbd30c

Browse files
SantiagoDePoloniaSantiagoDePolonia
committed
fix(execution-plans): address review feedback
1 parent 1b66997 commit 8cbd30c

9 files changed

Lines changed: 483 additions & 46 deletions

File tree

‎internal/admin/dashboard/static/css/dashboard.css‎

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2485,7 +2485,7 @@ body.conversation-drawer-open {
24852485
.ep-node-icon svg {
24862486
width: 15px;
24872487
height: 15px;
2488-
stroke: currentColor;
2488+
stroke: currentcolor;
24892489
fill: none;
24902490
stroke-width: 2;
24912491
stroke-linecap: round;
@@ -2655,17 +2655,6 @@ body.conversation-drawer-open {
26552655
gap: 6px;
26562656
}
26572657

2658-
.ep-node-ai .ep-node-icon {
2659-
width: 32px;
2660-
height: 32px;
2661-
border-radius: var(--radius);
2662-
}
2663-
2664-
.ep-node-ai .ep-node-icon svg {
2665-
width: 17px;
2666-
height: 17px;
2667-
}
2668-
26692658
/* AI skipped — cache hit absorbed the request */
26702659
.ep-node-ai-skipped {
26712660
opacity: 0.28;

‎internal/admin/dashboard/static/js/modules/execution-plans-layout.test.js‎

Lines changed: 41 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,39 @@ test('workflow nodes use endpoint and feature color groups consistently', () =>
9797
}
9898
});
9999

100+
test('execution plan authoring inputs expose stable accessible names', () => {
101+
const template = readFixture('../../../templates/index.html');
102+
103+
assert.match(
104+
template,
105+
/x-model="executionPlanFilter"[^>]*aria-label="Filter workflows by scope, name, hash, or guardrail"/
106+
);
107+
assert.match(
108+
template,
109+
/x-model="step\.ref"[^>]*aria-label="Guardrail reference"/
110+
);
111+
assert.match(
112+
template,
113+
/x-model\.number="step\.step"[^>]*aria-label="Guardrail step"/
114+
);
115+
});
116+
117+
test('guardrails node only renders a sublabel when step detail exists', () => {
118+
const template = readFixture('../../../templates/index.html');
119+
120+
assert.match(
121+
template,
122+
/<span class="ep-node-label">Guardrails<\/span>\s*<span class="ep-node-sub" x-show="epGuardrailLabel\(plan\)" x-text="epGuardrailLabel\(plan\)"><\/span>/
123+
);
124+
});
125+
126+
test('execution pipeline icons use lowercase currentcolor keyword', () => {
127+
const css = readFixture('../../css/dashboard.css');
128+
const iconRule = readCSSRule(css, '.ep-node-icon svg');
129+
130+
assert.match(iconRule, /stroke:\s*currentcolor;/);
131+
});
132+
100133
test('exec pipeline has bottom spacing so adjacent cards do not touch it', () => {
101134
const css = readFixture('../../css/dashboard.css');
102135
const pipelineRule = readCSSRule(css, '.exec-pipeline');
@@ -115,7 +148,6 @@ test('execution pipeline uses var(--radius) for chart-local corners', () => {
115148
'.ep-node-endpoint',
116149
'.ep-node-icon-endpoint',
117150
'.ep-node-ai',
118-
'.ep-node-ai .ep-node-icon',
119151
'.ep-node-async',
120152
'.ep-node-async .ep-node-icon'
121153
];
@@ -126,6 +158,14 @@ test('execution pipeline uses var(--radius) for chart-local corners', () => {
126158
}
127159
});
128160

161+
test('AI node renders as a text-only card without an icon', () => {
162+
const template = readFixture('../../../templates/index.html');
163+
const css = readFixture('../../css/dashboard.css');
164+
165+
assert.doesNotMatch(template, /class="ep-node ep-node-ai[^"]*"[^>]*>\s*<div class="ep-node-icon">/);
166+
assert.doesNotMatch(css, /\.ep-node-ai \.ep-node-icon\s*\{/);
167+
});
168+
129169
test('endpoint pills use dedicated flush-left icons and tighter right padding', () => {
130170
const template = readFixture('../../../templates/index.html');
131171
const css = readFixture('../../css/dashboard.css');

‎internal/admin/dashboard/static/js/modules/execution-plans.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -446,7 +446,7 @@
446446

447447
epGuardrailLabel(source) {
448448
const count = this.executionPlanSourceGuardrails(source).length;
449-
if (count === 0) return 'Guardrails';
449+
if (count === 0) return '';
450450
return count === 1 ? '1 step' : count + ' steps';
451451
},
452452

‎internal/admin/dashboard/static/js/modules/execution-plans.test.js‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -170,6 +170,40 @@ test('workflowDisplayName falls back to scope label or All models', () => {
170170
);
171171
});
172172

173+
test('epGuardrailLabel only shows a sublabel when guardrail steps exist', () => {
174+
const module = createExecutionPlansModule();
175+
176+
assert.equal(
177+
module.epGuardrailLabel({
178+
plan_payload: {
179+
guardrails: []
180+
}
181+
}),
182+
''
183+
);
184+
185+
assert.equal(
186+
module.epGuardrailLabel({
187+
plan_payload: {
188+
guardrails: [{ ref: 'policy-system', step: 10 }]
189+
}
190+
}),
191+
'1 step'
192+
);
193+
194+
assert.equal(
195+
module.epGuardrailLabel({
196+
plan_payload: {
197+
guardrails: [
198+
{ ref: 'policy-system', step: 10 },
199+
{ ref: 'pii', step: 20 }
200+
]
201+
}
202+
}),
203+
'2 steps'
204+
);
205+
});
206+
173207
test('deactivateExecutionPlan requires confirmation before posting', async () => {
174208
let fetchCalled = false;
175209
const module = createExecutionPlansModule({

‎internal/admin/dashboard/templates/index.html‎

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -635,7 +635,7 @@ <h2>Workflows</h2>
635635

636636
<div class="table-toolbar" x-show="executionPlansAvailable">
637637
<div class="table-toolbar-main">
638-
<input type="text" placeholder="Filter by scope, name, hash, or guardrail..." x-model="executionPlanFilter" class="filter-input">
638+
<input type="text" placeholder="Filter by scope, name, hash, or guardrail..." x-model="executionPlanFilter" class="filter-input" aria-label="Filter workflows by scope, name, hash, or guardrail">
639639
</div>
640640
<div class="table-toolbar-actions">
641641
<span class="model-count" x-text="filteredExecutionPlans.length + ' active scopes'"></span>
@@ -729,8 +729,8 @@ <h4>Guardrail Steps</h4>
729729
<div class="execution-plan-guardrail-list-editor" x-show="executionPlanForm.guardrails.length > 0">
730730
<template x-for="(step, index) in executionPlanForm.guardrails" :key="'execution-plan-form-guardrail-' + index">
731731
<div class="execution-plan-guardrail-row">
732-
<input type="text" class="filter-input execution-plan-input" list="execution-plan-guardrail-options" placeholder="Guardrail ref" x-model="step.ref">
733-
<input type="number" class="filter-input execution-plan-step-input" min="0" step="10" placeholder="Step" x-model.number="step.step">
732+
<input type="text" class="filter-input execution-plan-input" list="execution-plan-guardrail-options" placeholder="Guardrail ref" x-model="step.ref" aria-label="Guardrail reference">
733+
<input type="number" class="filter-input execution-plan-step-input" min="0" step="10" placeholder="Step" x-model.number="step.step" aria-label="Guardrail step">
734734
<button type="button" class="table-action-btn table-action-btn-danger" @click="removeExecutionPlanGuardrailStep(index)">Remove</button>
735735
</div>
736736
</template>
@@ -785,7 +785,7 @@ <h3 x-text="workflowDisplayName(plan)"></h3>
785785
<svg viewBox="0 0 24 24"><path d="M12 22s8-4 8-10V5l-8-3-8 3v7c0 6 8 10 8 10z"/></svg>
786786
</div>
787787
<span class="ep-node-label">Guardrails</span>
788-
<span class="ep-node-sub" x-text="epGuardrailLabel(plan)"></span>
788+
<span class="ep-node-sub" x-show="epGuardrailLabel(plan)" x-text="epGuardrailLabel(plan)"></span>
789789
</div>
790790
</div>
791791

@@ -806,9 +806,6 @@ <h3 x-text="workflowDisplayName(plan)"></h3>
806806

807807
<!-- AI node (centered by equal halves) -->
808808
<div class="ep-node ep-node-ai">
809-
<div class="ep-node-icon">
810-
<svg viewBox="0 0 24 24"><polygon points="12 2 15.09 8.26 22 9.27 17 14.14 18.18 21.02 12 17.77 5.82 21.02 7 14.14 2 9.27 8.91 8.26 12 2"/></svg>
811-
</div>
812809
<span class="ep-node-label" x-text="epAiLabel(plan, null)"></span>
813810
<span class="ep-node-sub" x-show="epAiSublabel(plan, null)" x-text="epAiSublabel(plan, null)"></span>
814811
</div>
@@ -1022,9 +1019,6 @@ <h2>Audit Logs</h2>
10221019

10231020
<!-- AI node (centered by equal halves) -->
10241021
<div class="ep-node ep-node-ai" :class="epAiNodeClass(epRuntimeFromEntry(entry))">
1025-
<div class="ep-node-icon">
1026-
<svg viewBox="0 0 24 24"><polygon points="12 2 15.09 8.26 22 9.27 17 14.14 18.18 21.02 12 17.77 5.82 21.02 7 14.14 2 9.27 8.91 8.26 12 2"/></svg>
1027-
</div>
10281022
<span class="ep-node-label" x-text="epAiLabel(null, epRuntimeFromEntry(entry))"></span>
10291023
<span class="ep-node-sub" x-show="epAiSublabel(null, epRuntimeFromEntry(entry))" x-text="epAiSublabel(null, epRuntimeFromEntry(entry))"></span>
10301024
</div>

‎internal/admin/handler.go‎

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -683,6 +683,10 @@ func (h *Handler) CreateExecutionPlan(c *echo.Context) error {
683683
return handleError(c, core.NewInvalidRequestError("invalid request body: "+err.Error(), err))
684684
}
685685

686+
if err := h.validateExecutionPlanScope(req.ScopeProvider, req.ScopeModel); err != nil {
687+
return handleError(c, err)
688+
}
689+
686690
if err := h.validateExecutionPlanGuardrails(req.Payload); err != nil {
687691
return handleError(c, err)
688692
}
@@ -750,6 +754,34 @@ func (h *Handler) validateExecutionPlanGuardrails(payload executionplans.Payload
750754
return nil
751755
}
752756

757+
func (h *Handler) validateExecutionPlanScope(scopeProvider, scopeModel string) error {
758+
scopeProvider = strings.TrimSpace(scopeProvider)
759+
scopeModel = strings.TrimSpace(scopeModel)
760+
761+
if scopeProvider == "" {
762+
if scopeModel != "" {
763+
return core.NewInvalidRequestError("scope_model requires scope_provider", nil)
764+
}
765+
return nil
766+
}
767+
if h.registry == nil {
768+
return core.NewInvalidRequestError("provider registry is unavailable for workflow scope validation", nil)
769+
}
770+
if !slices.Contains(h.registry.ProviderTypes(), scopeProvider) {
771+
return core.NewInvalidRequestError("unknown provider type: "+scopeProvider, nil)
772+
}
773+
if scopeModel == "" {
774+
return nil
775+
}
776+
777+
for _, model := range h.registry.ListModelsWithProvider() {
778+
if model.ProviderType == scopeProvider && model.Model.ID == scopeModel {
779+
return nil
780+
}
781+
}
782+
return core.NewInvalidRequestError("unknown model for provider "+scopeProvider+": "+scopeModel, nil)
783+
}
784+
753785
func decodeAliasPathName(raw string) (string, error) {
754786
name, err := url.PathUnescape(strings.TrimSpace(raw))
755787
if err != nil {

0 commit comments

Comments
 (0)