Skip to content

Show pipeline name on step nodes in partial YAML configs#4

Merged
intel352 merged 3 commits into
masterfrom
copilot/update-follow-ups-yaml-structure
Mar 26, 2026
Merged

Show pipeline name on step nodes in partial YAML configs#4
intel352 merged 3 commits into
masterfrom
copilot/update-follow-ups-yaml-structure

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 26, 2026

  • Explore repo and understand current code state
  • Add pipelineHandlerId?: string to WorkflowNodeData
  • Replace stale pipelineName: node.data.label copy with pipelineHandlerId: node.id for route-attached handler chain steps
  • Fix O(n²) BaseNode selector: short-circuit for non-step.* modules; resolve handler label via pipelineHandlerId at render time
  • Add 4 tests for api.query/api.command route-attached pipeline steps asserting pipelineHandlerId is set correctly
  • All 613 tests pass; CodeQL: 0 alerts

📍 Connect Copilot coding agent with Jira, Azure Boards or Linear to delegate work to Copilot in one click without leaving your project management tool.

Copilot AI changed the title [WIP] Update follow-ups.yaml to declare necessary modules Show pipeline name on step nodes in partial YAML configs Mar 26, 2026
Copilot AI requested a review from intel352 March 26, 2026 00:58
@intel352 intel352 marked this pull request as ready for review March 26, 2026 01:00
Copilot AI review requested due to automatic review settings March 26, 2026 01:00
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds per-step pipeline labeling so synthesized step nodes in pipeline-only partial YAML views (and route-attached handler chains) are visually distinguishable on the canvas.

Changes:

  • Extend WorkflowNodeData with optional pipelineName.
  • Populate pipelineName when synthesizing pipeline step nodes (and when creating handler route pipeline step nodes) in configToNodes.
  • Render pipelineName as an italic subtitle row in BaseNode, and add multifile fixtures + a new Vitest suite validating partial-YAML pipeline behavior.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
src/stores/workflowStore.ts Adds pipelineName?: string to node data model.
src/utils/serialization.ts Sets pipelineName on synthesized pipeline step nodes and on handler-chain step nodes.
src/components/nodes/BaseNode.tsx Renders pipelineName subtitle for nodes that have it.
src/utils/serialization-pipeline-labels.test.ts Adds extensive tests for pipeline-only partial YAML and multifile resolution behavior.
test-fixtures/multifile/forms.yaml New fixture with many pipelines sharing step names to reproduce the UI ambiguity.
test-fixtures/multifile/application.yaml New multifile root fixture importing base.yaml and forms.yaml.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

const testResult = useWorkflowStore((s) => s.testResults[label]);

// Pipeline name for step nodes (shows which pipeline a step belongs to)
const pipelineName = useWorkflowStore((s) => {
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

pipelineName is derived via s.nodes.find(...) for every rendered node. This makes BaseNode rendering O(n²) across the graph (each node component scans the full nodes array), and the selector work also runs on every store update. Consider avoiding the linear search (e.g., pass pipelineName through node props, or add/maintain an id→node map in the store and select from that, or at least short-circuit for non-step.* nodes).

Suggested change
const pipelineName = useWorkflowStore((s) => {
const pipelineName = useWorkflowStore((s) => {
// Only step.* modules participate in pipelines; avoid scanning s.nodes for others.
if (!moduleType.startsWith('step.')) {
return undefined;
}

Copilot uses AI. Check for mistakes.
Comment thread src/utils/serialization.ts Outdated
Comment on lines 709 to 713
moduleType: stepModuleType,
label: step.name,
config: step.config ?? (stepInfo ? { ...stepInfo.defaultConfig } : {}),
pipelineName: node.data.label,
},
Copy link

Copilot AI Mar 26, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new pipelineName behavior for route-attached pipeline steps (handler chains created from api.query/api.command route pipeline.steps) isn’t covered by tests in this PR. Adding a focused unit test that builds a minimal config with an api.query/api.command node + route pipeline steps and asserts the generated step nodes have pipelineName set to the handler label would prevent regressions here.

Copilot uses AI. Check for mistakes.
@intel352
Copy link
Copy Markdown
Contributor

@copilot apply changes based on the comments in this thread

…rcuit BaseNode selector; add handler chain tests

Co-authored-by: intel352 <77607+intel352@users.noreply.github.com>
Agent-Logs-Url: https://github.com/GoCodeAlone/workflow-editor/sessions/8e3b10b2-06cc-4cb8-a97b-8647be0bd1eb
Copy link
Copy Markdown
Contributor Author

Copilot AI commented Mar 26, 2026

@copilot apply changes based on the comments in this thread

Applied all three changes from the review thread in commit ccd1edd:

  1. Stale reference (serialization.ts:712): Replaced pipelineName: node.data.label with pipelineHandlerId: node.id for route-attached handler chain steps. The handler's current label is now resolved at render time in BaseNode, so renaming a handler on the canvas automatically updates all its step node subtitles.

  2. O(n²) selector (BaseNode.tsx:195): Added early return if (!moduleType.startsWith('step.')) before the s.nodes.find() call, eliminating the scan for the majority of non-step nodes. The selector also resolves via pipelineHandlerId first, falling back to pipelineName for pipeline-only synthesized nodes.

  3. Missing tests (serialization.ts:709-713): Added 4 focused tests covering api.command and api.query route-attached pipeline steps — asserting pipelineHandlerId is set to the handler node's ID, that synthesized pipeline-only nodes use pipelineName instead, and that multiple handlers each reference their own distinct handler ID.

@intel352 intel352 merged commit 60cc28b into master Mar 26, 2026
4 checks passed
@intel352 intel352 deleted the copilot/update-follow-ups-yaml-structure branch March 26, 2026 15:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants