docs: add design and implementation revision documents

Revision docs correct original plans: schedule persistence via API endpoints (not tree_structure), array-index reorder (no display_order), store minimum-one-step invariant, accordion mode, ARIA requirements, and two-stage save orchestration with failure handling. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-02-19 07:02:47 -05:00
parent ef699f3eab
commit 7f520772ec
2 changed files with 380 additions and 0 deletions
--- a/docs/plans/2026-02-19-procedural-editor-redesign-design-revisions.md
+++ b/docs/plans/2026-02-19-procedural-editor-redesign-design-revisions.md
@@ -0,0 +1,155 @@
+# Procedural & Maintenance Editor Redesign - Design Revisions
+
+> **Date:** 2026-02-19
+> **Revises:** `docs/plans/2026-02-19-procedural-editor-redesign-design.md`
+> **Purpose:** Resolve implementation gaps and make the design decision-complete for engineering handoff
+
+## Summary
+
+This revision tightens the original design to match current architecture and APIs.
+It resolves contradictions around maintenance schedule persistence, aligns step-list behavior with store invariants, and defines concrete DnD/accessibility/test expectations.
+
+## Revision Decisions (Locked)
+
+1. Keep fixed-height editor layout with independent StepList scrolling.
+2. Use collapsible sections for Details, Intake Form, and Maintenance Schedule.
+3. Use existing installed `@dnd-kit/*` packages (no new dependency work).
+4. Keep `procedure_end` as non-draggable and always last.
+5. Keep schedule as optional for maintenance flows (manual batch launch remains valid).
+
+## Critical Corrections to Original Design
+
+1. **Maintenance schedule persistence**
+- Schedule is not embedded in `tree_structure`.
+- Schedule must be persisted through maintenance schedule endpoints.
+- New unsaved flow requires two-stage save:
+  1. Save tree (`treesApi.create` / `treesApi.update`)
+  2. Create/update schedule (`maintenanceSchedulesApi.create` or `maintenanceSchedulesApi.update`)
+- One schedule per tree (`uq_maintenance_schedules_tree_id`).
+
+2. **Step empty-state semantics**
+- Original "0 steps" state conflicts with current store minimum step behavior.
+- Revised behavior:
+  - If minimum-one-step invariant remains, remove "0 steps" UX language.
+  - Empty-state is only shown if invariant is intentionally changed in store.
+
+3. **DnD data model correction**
+- Do not mention recalculating `display_order` for procedural steps.
+- Reorder is array-index based only.
+- Explicitly block dragging `procedure_end`.
+
+4. **Dependencies section correction**
+- Replace "New Dependencies" with "Existing Dependencies Used" for `@dnd-kit/core`, `@dnd-kit/sortable`, `@dnd-kit/utilities`.
+
+5. **File impact correction**
+- Add API integration surfaces:
+  - `frontend/src/api/maintenanceSchedules.ts`
+  - target-list integration file(s) if inline list creation is in scope
+- Clarify interaction with `frontend/src/pages/MaintenanceFlowDetailPage.tsx` (retain schedule view/edit there or shift entirely to editor).
+
+6. **Collapsible behavior clarification**
+- Use single-open accordion mode by default.
+- Details defaults expanded for new flows.
+- Intake defaults collapsed.
+- Maintenance Schedule defaults:
+  - new maintenance flow: expanded
+  - existing with schedule: collapsed summary
+  - existing without schedule: collapsed with "Set Up" affordance
+
+7. **A11y + keyboard DnD acceptance**
+- Include keyboard reorder acceptance criteria.
+- Include focus order and ARIA labeling criteria for section toggles and drag handles.
+
+## Revised Maintenance Schedule Section Spec
+
+### New maintenance flow (tree not yet saved)
+
+1. Render schedule editor expanded.
+2. Keep schedule input in local UI draft state.
+3. On Save Draft / Publish:
+- Save tree first.
+- If tree save succeeds, create schedule with resulting `tree_id`.
+4. If schedule create fails:
+- Tree save remains successful.
+- Show actionable error ("Schedule not saved. Retry.").
+- Preserve schedule draft state.
+
+### Existing maintenance flow
+
+1. Load schedule via `maintenanceSchedulesApi.getForTree(treeId)`.
+2. Edit and persist via `maintenanceSchedulesApi.update(scheduleId, data)`.
+3. If no schedule exists, show collapsed "No schedule configured" summary + setup button.
+
+## Revised StepList Reorder Spec
+
+1. Draggable types:
+- `procedure_step`
+- `section_header`
+2. Non-draggable:
+- `procedure_end`
+3. Reorder behavior:
+- Move item by array index.
+- Preserve all step payload fields.
+- No implicit grouped movement under section headers.
+4. Keyboard behavior:
+- Drag handle focusable.
+- Enter/Space pick up + drop.
+- Arrow keys move while grabbed.
+- Escape cancels.
+
+## Acceptance Criteria
+
+1. **Layout**
+- Toolbar remains visible while StepList scrolls.
+- Details/Intake/Schedule sections collapse without shrinking StepList usability.
+
+2. **Steps**
+- New step auto-expands.
+- New step scrolls into view.
+- Reorder works by pointer and keyboard.
+- `procedure_end` remains last and fixed.
+
+3. **Maintenance schedule**
+- New unsaved maintenance flow can be saved without schedule.
+- Schedule can be created immediately after first tree save.
+- Existing schedule loads and updates in editor.
+- Schedule failure does not roll back successful tree save.
+
+4. **Accessibility**
+- Section toggles keyboard operable.
+- Drag handles have accessible labels.
+- Focus remains stable after reorder.
+
+5. **Build/Test**
+- `npm run build` succeeds.
+- Affected component/store tests pass.
+
+## Updated File Impact
+
+### Modified
+
+- `frontend/src/pages/ProceduralEditorPage.tsx`
+- `frontend/src/components/procedural-editor/StepList.tsx`
+- `frontend/src/components/procedural-editor/IntakeFormBuilder.tsx`
+- `frontend/src/store/proceduralEditorStore.ts`
+- `frontend/src/api/maintenanceSchedules.ts`
+- `frontend/src/pages/MaintenanceFlowDetailPage.tsx` (if schedule UX ownership changes)
+
+### New
+
+- `frontend/src/components/procedural-editor/CollapsibleEditorSection.tsx`
+- `frontend/src/components/procedural-editor/MaintenanceScheduleSection.tsx`
+
+### Existing dependencies used
+
+- `@dnd-kit/core`
+- `@dnd-kit/sortable`
+- `@dnd-kit/utilities`
+
+## Out of Scope (unchanged)
+
+1. Intake field DnD reorder.
+2. Procedural undo/redo parity.
+3. Step templates/presets.
+4. Bulk step operations.
+5. Backend schema/model changes for procedural steps.
--- a/docs/plans/2026-02-19-procedural-editor-redesign-impl-revisions.md
+++ b/docs/plans/2026-02-19-procedural-editor-redesign-impl-revisions.md
@@ -0,0 +1,225 @@
+# Procedural Editor Redesign - Implementation Revisions
+
+> **Date:** 2026-02-19
+> **Revises:** `docs/plans/2026-02-19-procedural-editor-redesign-impl.md`
+> **Related Design Revision:** `docs/plans/2026-02-19-procedural-editor-redesign-design-revisions.md`
+
+## Goal
+
+Revise the implementation plan so it matches actual architecture and APIs, with explicit handling for maintenance schedule persistence, step-list invariants, DnD constraints, and accessibility/test requirements.
+
+## Critical Corrections from Original Impl Plan
+
+1. Do not treat maintenance schedule as part of `tree_structure`.
+2. Do not use `display_order` for procedural step reorder.
+3. Do not assume `0 steps` state unless store invariant is changed intentionally.
+4. Do not list `@dnd-kit/*` as new dependency (already installed).
+5. Add explicit save orchestration for unsaved maintenance flows.
+6. Add explicit failure handling when tree save succeeds but schedule save fails.
+
+## Phase 0: Scope Lock
+
+### Task 0.1 - Confirm invariants and UX ownership
+
+**Decisions to lock in code before implementation:**
+1. `procedure_end` remains fixed, non-draggable, last.
+2. Minimum one `procedure_step` remains enforced (recommended).
+3. Schedule editing in editor is source-of-truth for create/edit, with detail page as display/secondary entrypoint.
+
+**Files:** none (decision checkpoint)
+
+---
+
+## Phase 1: Layout and Collapsible Sections
+
+### Task 1.1 - Add shared collapsible wrapper
+
+**Files:**
+- Create: `frontend/src/components/procedural-editor/CollapsibleEditorSection.tsx`
+
+**Requirements:**
+1. Single-row collapsed summary.
+2. Keyboard-accessible toggle button.
+3. `aria-expanded`, `aria-controls`, and section `id`.
+4. Optional `defaultExpanded`.
+
+### Task 1.2 - Convert ProceduralEditorPage to fixed-height editor
+
+**Files:**
+- Modify: `frontend/src/pages/ProceduralEditorPage.tsx`
+
+**Changes:**
+1. Outer layout becomes `flex h-full flex-col overflow-hidden`.
+2. Toolbar becomes sticky.
+3. Details and Intake wrapped in `CollapsibleEditorSection`.
+4. Steps area becomes `flex-1 min-h-0 overflow-y-auto`.
+5. Accordion mode: only one section open at a time (explicit state in page component).
+
+**Summaries:**
+1. Details: `"Name" - N tags - Public/Private`.
+2. Intake: `N fields: label1, label2...` (truncate).
+
+---
+
+## Phase 2: StepList Behavior and DnD
+
+### Task 2.1 - Align header/empty behavior with current store invariant
+
+**Files:**
+- Modify: `frontend/src/components/procedural-editor/StepList.tsx`
+- Optional invariant change (if desired): `frontend/src/store/proceduralEditorStore.ts`
+
+**Required behavior (recommended):**
+1. Keep minimum one `procedure_step`.
+2. Remove/unset any `0 steps` UI paths.
+3. Header shows:
+- `Steps (N steps - ~M min)` when estimates exist
+- `Steps (N steps)` otherwise.
+
+### Task 2.2 - Ensure new step auto-expands + scrolls into view
+
+**Files:**
+- Modify: `frontend/src/components/procedural-editor/StepList.tsx`
+- Verify existing store behavior in `frontend/src/store/proceduralEditorStore.ts`
+
+**Behavior:**
+1. On add step/section, expanded editor opens immediately.
+2. Newly inserted row is scrolled into view via stable element refs (prefer scroll target by id over "scroll to bottom").
+
+### Task 2.3 - Implement DnD with current model constraints
+
+**Files:**
+- Modify: `frontend/src/components/procedural-editor/StepList.tsx`
+- Modify: `frontend/src/store/proceduralEditorStore.ts` (reuse `moveStep`)
+
+**Rules:**
+1. Draggable: `procedure_step`, `section_header`.
+2. Non-draggable: `procedure_end`.
+3. Reorder by array index only.
+4. No `display_order` recalculation for steps.
+5. Keyboard drag support and visible insertion indicator.
+
+---
+
+## Phase 3: Maintenance Schedule Section (Correct API orchestration)
+
+### Task 3.1 - Add schedule section component
+
+**Files:**
+- Create: `frontend/src/components/procedural-editor/MaintenanceScheduleSection.tsx`
+- Modify: `frontend/src/pages/ProceduralEditorPage.tsx`
+
+**Behavior:**
+1. Render only for `treeType === 'maintenance'`.
+2. Capture:
+- cron expression
+- timezone
+- target list id
+3. Collapsed summary:
+- configured: human-readable cadence + target list status
+- unconfigured: `No schedule configured`.
+
+### Task 3.2 - Add schedule draft UI state and save orchestration
+
+**Files:**
+- Modify: `frontend/src/store/proceduralEditorStore.ts` (UI draft state only)
+- Modify: `frontend/src/pages/ProceduralEditorPage.tsx`
+- Use: `frontend/src/api/maintenanceSchedules.ts`
+
+**Save flow:**
+1. Save tree first (`create`/`update`).
+2. If maintenance and schedule draft present:
+- if existing schedule id: `maintenanceSchedulesApi.update`
+- else: `maintenanceSchedulesApi.create` with saved tree id.
+3. If schedule save fails:
+- keep tree save success
+- show actionable error toast/banner
+- preserve schedule draft as dirty.
+
+### Task 3.3 - Existing flow load
+
+**Files:**
+- Modify: `frontend/src/pages/ProceduralEditorPage.tsx`
+
+**Behavior:**
+1. On edit maintenance flow, fetch schedule via `getForTree(treeId)`.
+2. 404 = no schedule yet (valid state).
+3. Hydrate schedule draft state for section UI.
+
+---
+
+## Phase 4: Integration polish and consistency
+
+### Task 4.1 - Clarify MaintenanceFlowDetailPage role
+
+**Files:**
+- Modify (if needed): `frontend/src/pages/MaintenanceFlowDetailPage.tsx`
+
+**Decision implementation:**
+1. Keep schedule read-only there, with "Edit in Flow Editor" CTA.
+2. Avoid split-brain schedule edits in two places unless explicitly desired.
+
+---
+
+## Phase 5: Tests and verification
+
+### Task 5.1 - Automated tests
+
+**Files (new/updated):**
+- `frontend/src/components/procedural-editor/StepList.test.tsx`
+- `frontend/src/pages/ProceduralEditorPage.test.tsx`
+- `frontend/src/store/proceduralEditorStore.test.ts` (if absent, add focused tests)
+
+**Minimum coverage:**
+1. Reorder respects `procedure_end` constraints.
+2. New steps auto-expand and scroll target call occurs.
+3. Accordion open/close state and summaries.
+4. Maintenance save orchestration:
+- tree create/update then schedule create/update
+- schedule failure does not revert tree success.
+
+### Task 5.2 - Manual acceptance checklist
+
+1. Steps list remains primary viewport focus in fixed-height layout.
+2. Details/Intake/Schedule sections collapse and summarize correctly.
+3. DnD works by mouse and keyboard.
+4. End step never drags.
+5. New maintenance flow:
+- can save draft without schedule
+- can save with schedule in one action (tree first, schedule second).
+6. Existing maintenance flow loads schedule and saves edits.
+
+### Task 5.3 - Build and lint gates
+
+1. `cd frontend && npm run build`
+2. `cd frontend && npm run test`
+3. `cd frontend && npm run lint`
+
+---
+
+## File Impact (Revised)
+
+### Create
+1. `frontend/src/components/procedural-editor/CollapsibleEditorSection.tsx`
+2. `frontend/src/components/procedural-editor/MaintenanceScheduleSection.tsx`
+
+### Modify
+1. `frontend/src/pages/ProceduralEditorPage.tsx`
+2. `frontend/src/components/procedural-editor/StepList.tsx`
+3. `frontend/src/components/procedural-editor/IntakeFormBuilder.tsx`
+4. `frontend/src/store/proceduralEditorStore.ts`
+5. `frontend/src/pages/MaintenanceFlowDetailPage.tsx` (if ownership adjusted)
+
+### Existing APIs used
+1. `frontend/src/api/maintenanceSchedules.ts`
+2. target list API module(s) if inline list selection/creation is implemented
+
+---
+
+## Out of Scope (unchanged)
+
+1. Intake field DnD reorder.
+2. Procedural undo/redo parity.
+3. Step templates/presets.
+4. Bulk step operations.
+5. Backend schema/model changes for procedural steps.