Skip to content

chore(#760): LOW tier — remove old skills, fix direction ternary (#735, #738)#775

Merged
amiable-dev merged 2 commits intomainfrom
fix/760-low-tier
Mar 23, 2026
Merged

chore(#760): LOW tier — remove old skills, fix direction ternary (#735, #738)#775
amiable-dev merged 2 commits intomainfrom
fix/760-low-tier

Conversation

@amiable-dev
Copy link
Owner

Summary

LOW tier cleanup from #760 quality review backlog:

#738: Remove deprecated skill directories

  • Delete skills/conductor-device-setup/ (replaced by conductor-binding-setup)
  • Delete skills/conductor-midi-learn/ (replaced by conductor-learn)
  • 1,250 lines removed

#735: DeviceEditor direction label cleanup

  • Replace inline ternary chain with DIR_LABELS lookup map
  • Consistent with DeviceDetail.svelte and DirectionBadge.svelte patterns
  • 26 DeviceEditor tests pass

Remaining LOW items — intentionally deferred

Most remaining LOW items are either:

  • Intentional design decisions: DeviceGroup component name kept per D1, MIDI_LEARN workspace key still active
  • Pre-existing issues: skill validator allowed-tools pattern, L1 inline vs file load
  • Placeholder behavior: Create Binding button, binding editor click, OSC/ArtNet groups
  • Future work: protocol enforcement, channel scope in description

These don't warrant fixes — they're documented in #760 as known limitations.

#760 is now substantially complete

Phase Status PRs
Phase 1: ADR-023 Done #768
Phase 2: CRITICAL (6) Done #769, #770, #771
Phase 3: HIGH (5) Done #772, #773
Phase 4: MEDIUM (3 backend) Done #774
Phase 5: LOW (2) This PR #775

🤖 Generated with Claude Code

amiable-dev and others added 2 commits March 23, 2026 16:25
conductor-device-setup → conductor-binding-setup (ADR-022 D9)
conductor-midi-learn → conductor-learn (ADR-022 A2.3)

Old directories retained for backward compat during migration,
now safe to remove since the new skills are in place.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ap (#735)

Consistent with DeviceDetail.svelte and DirectionBadge.svelte patterns.
Easier to extend if a 4th direction value is added.

26 DeviceEditor tests pass.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings March 23, 2026 16:28
Copy link
Contributor

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

This PR completes two LOW-tier cleanup items from the #760 backlog by removing deprecated agent skill directories and simplifying the DeviceEditor direction label rendering in the GUI.

Changes:

  • Removed deprecated skills conductor-device-setup and conductor-midi-learn (and their reference docs) now superseded by conductor-binding-setup / conductor-learn.
  • Replaced an inline direction-label ternary chain in DeviceEditor.svelte with a DIR_LABELS lookup map for consistent label rendering.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
skills/conductor-midi-learn/SKILL.md Deleted deprecated Learn skill (superseded by conductor-learn).
skills/conductor-midi-learn/references/PATTERNS.md Deleted deprecated pattern reference doc for the removed Learn skill.
skills/conductor-device-setup/SKILL.md Deleted deprecated setup skill (superseded by conductor-binding-setup).
skills/conductor-device-setup/references/DEVICES.md Deleted deprecated supported-devices reference doc for the removed setup skill.
conductor-gui/ui/src/lib/components/DeviceEditor.svelte Introduced DIR_LABELS map to render direction labels without a nested ternary.

@amiable-dev amiable-dev requested a review from Copilot March 23, 2026 17:12
@amiable-dev amiable-dev merged commit fc1818d into main Mar 23, 2026
16 checks passed
Copy link
Contributor

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

Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.

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.

2 participants