docs: Refine implementation plan and document draft feature

- Updated IMPLEMENTATION-PLAN-STEP-LIBRARY-FRONTEND.md with design decisions: - Custom steps persistence: separate `custom_steps` field in sessions - Custom step navigation: full step type support (decision/action/solution) - Validation warnings: inline dismissible, no confirmation modal - Added backend migration task (B.10) for custom_steps field - Updated file count: 10 new, 8 modified, 1 migration - Clarified acceptance criteria for validation behavior - Created docs/plans/2026-02-03-draft-trees-feature.md: - Comprehensive design for draft trees and custom steps - Database schema, API changes, frontend UX patterns - Implementation phases and success metrics - Related to Issue #25 (planned for Phase 3) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
2026-02-03 18:51:27 -05:00
parent 67a98bc25c
commit 4378ec4b20
2 changed files with 744 additions and 46 deletions
--- a/docs/IMPLEMENTATION-PLAN-STEP-LIBRARY-FRONTEND.md
+++ b/docs/IMPLEMENTATION-PLAN-STEP-LIBRARY-FRONTEND.md
@@ -1,9 +1,29 @@
 # Implementation Plan: Step Library Frontend + Tree Editor Validation

-> **Date:** February 3, 2026
+> **Date:** February 3, 2026 (Updated after refinement session)
 > **Scope:** Issues #1, #8, #9, #10
-> **Estimated Components:** 8 new files, 4 modified files
+> **Estimated Components:** 10 new files, 8 modified files, 1 migration
 > **Parallel Workstreams:** 2
+> **Related Planning:** Issue #25 (Draft Trees - Phase 3)
+
+---
+
+## Plan Refinements (Feb 3, 2026)
+
+This plan was refined through collaborative design discussion. Key changes from initial draft:
+
+1. **Custom Steps Persistence** - Changed from storing in `decisions` array to separate `custom_steps` JSONB field
+   - Added: Backend migration task (B.10)
+   - Added: Session schema updates
+   - Enables: Session resuming with custom steps
+
+2. **Custom Step Navigation** - Expanded to support all step types (decision/action/solution)
+   - Added: Detailed rendering logic for each type in B.11
+   - Enables: Custom branching decisions during troubleshooting
+
+3. **Validation Warnings** - Clarified as inline/dismissible (no confirmation modal)
+   - Updated: Acceptance criteria to specify warning behavior
+   - Deferred: "Save as Draft" feature to Issue #25 (Phase 3)

 ---

@@ -108,10 +128,11 @@ Features:

 Modifications:
 1. Call `validate()` before save attempt
-2. Show ValidationSummary when errors exist
-3. Block save if any severity='error' exists (warnings allow save)
-4. Add "Validate" button in toolbar for manual check
-5. Auto-validate on blur from form fields (debounced)
+2. Show ValidationSummary when errors or warnings exist
+3. Block save if any severity='error' exists (disable save button)
+4. Warnings are **informational only** - do not block save, no confirmation modal
+5. Add "Validate" button in toolbar for manual check
+6. Auto-validate on blur from form fields (debounced)

 #### A.4: Visual Node Error Indicators
 **File:** `frontend/src/components/tree-editor/NodeList.tsx`
@@ -123,13 +144,14 @@ Modifications:

 ### Acceptance Criteria - Workstream A

- [ ] Cannot save tree without a name
- [ ] Cannot save tree without at least one solution node
+- [ ] Cannot save tree without a name (error blocks save)
+- [ ] Cannot save tree without at least one solution node (error blocks save)
 - [ ] Validation errors display in a clear, clickable list
 - [ ] Clicking an error selects the problematic node
- [ ] Circular references are detected and blocked
- [ ] Orphan nodes show as warnings (allow save with confirmation)
+- [ ] Circular references are detected and blocked (error)
+- [ ] Orphan nodes show as warnings (informational, do not block save)
 - [ ] Save button disabled when errors exist
+- [ ] Warnings are dismissible/ignorable - user can save with warnings present

 ---

@@ -387,7 +409,66 @@ interface CustomStepModalProps {

 ### Issue #8: Add Custom Step Button in Tree Navigation

-#### B.10: Modify TreeNavigationPage
+#### B.10: Add Custom Steps Backend Support
+**Files:**
+- `backend/alembic/versions/XXXX_add_custom_steps_to_sessions.py` (NEW - migration)
+- `backend/app/schemas/session.py` (MODIFIED)
+- `backend/app/api/endpoints/sessions.py` (MODIFIED)
+
+**Migration:**
+```python
+def upgrade():
+    # Add custom_steps JSONB column to sessions table
+    op.add_column('sessions',
+        sa.Column('custom_steps', JSONB, nullable=False, server_default='[]')
+    )
+
+def downgrade():
+    op.drop_column('sessions', 'custom_steps')
+```
+
+**Schema Updates:**
+```python
+# In DecisionRecord - no changes needed (still uses node_id)
+
+# In SessionResponse - add custom_steps field
+class SessionResponse(BaseModel):
+    # ... existing fields
+    custom_steps: list[dict[str, Any]] = Field(default_factory=list)  # NEW
+
+# In SessionUpdate - add custom_steps field
+class SessionUpdate(BaseModel):
+    # ... existing fields
+    custom_steps: Optional[list[dict[str, Any]]] = None  # NEW
+```
+
+**Custom Step Structure in Database:**
+```python
+# Each item in custom_steps array:
+{
+    "id": "uuid-string",
+    "inserted_after_node_id": "parent-node-id",
+    "step_data": {
+        "title": "Check Additional Logs",
+        "step_type": "action",  # decision | action | solution
+        "content": {
+            "instructions": "Check /var/log/...",
+            "help_text": "Optional help",
+            "commands": [...]  # optional
+        },
+        "category_id": "optional-uuid",
+        "tags": ["optional"]
+    },
+    "timestamp": "2026-02-03T10:30:00Z"
+}
+```
+
+**API Logic:**
+- `PUT /api/v1/sessions/{id}` - Accept `custom_steps` in request body
+- Validation: Ensure `step_data.step_type` is valid enum value
+- No cascade delete needed - custom steps are session-scoped
+
+#### B.11: Modify TreeNavigationPage
 **File:** `frontend/src/pages/TreeNavigationPage.tsx`

 Add state:
@@ -413,31 +494,83 @@ const [customSteps, setCustomSteps] = useState<CustomStep[]>([])
 3. Handle insert:
 ```typescript
 const handleInsertCustomStep = (step: Step | CustomStepDraft) => {
-  // Insert after current node in session
+  // Create custom step object
  const customStep: CustomStep = {
    id: crypto.randomUUID(),
    inserted_after_node_id: currentNodeId,
    step_data: step,
    timestamp: new Date().toISOString()
  }
-  setCustomSteps([...customSteps, customStep])
+
+  // Add to local state
+  const newCustomSteps = [...customSteps, customStep]
+  setCustomSteps(newCustomSteps)

  // Navigate to custom step (becomes current)
  setCurrentNodeId(customStep.id)
  setShowCustomStepModal(false)
+
+  // Save to backend
+  if (session) {
+    sessionsApi.update(session.id, {
+      custom_steps: newCustomSteps
+    }).catch(err => console.error('Failed to save custom step:', err))
+  }
 }
 ```

-4. Render custom steps in the navigation flow with visual indicator:
+4. Render custom steps based on type:
 ```tsx
-{/* Custom Step Badge */}
-<span className="rounded-full bg-purple-100 px-2 py-1 text-xs font-medium text-purple-800">
-  Custom Step
-</span>
+{/* Render custom step in navigation flow */}
+{currentNode?.id.startsWith('custom-') && (
+  <div className="rounded-lg border border-purple-200 bg-purple-50 p-4 dark:border-purple-800 dark:bg-purple-900/20">
+    {/* Custom Step Badge */}
+    <span className="mb-2 inline-block rounded-full bg-purple-100 px-2 py-1 text-xs font-medium text-purple-800 dark:bg-purple-900 dark:text-purple-100">
+      Custom Step
+    </span>
+
+    {/* Render based on step type */}
+    {customStepNode.step_data.step_type === 'decision' && (
+      <>
+        <h2>{customStepNode.step_data.title}</h2>
+        <MarkdownContent content={customStepNode.step_data.content.instructions} />
+        {/* Render decision options - user creates them inline or uses predefined */}
+        <div className="mt-4 space-y-2">
+          <button onClick={() => handleCustomDecisionOption('yes')}>Yes</button>
+          <button onClick={() => handleCustomDecisionOption('no')}>No</button>
+        </div>
+      </>
+    )}
+
+    {customStepNode.step_data.step_type === 'action' && (
+      <>
+        <h2>{customStepNode.step_data.title}</h2>
+        <MarkdownContent content={customStepNode.step_data.content.instructions} />
+        {customStepNode.step_data.content.commands?.map(cmd => (
+          <pre key={cmd.label}>{cmd.command}</pre>
+        ))}
+        <button onClick={handleContinueFromCustomAction}>Continue</button>
+      </>
+    )}
+
+    {customStepNode.step_data.step_type === 'solution' && (
+      <>
+        <h2>{customStepNode.step_data.title}</h2>
+        <MarkdownContent content={customStepNode.step_data.content.instructions} />
+        <button onClick={handleComplete}>Complete Session</button>
+      </>
+    )}
+  </div>
+)}
 ```

-#### B.11: Update Session Export
-**File:** `frontend/src/components/session/ExportPreviewModal.tsx`
+**Navigation Logic:**
+- Custom decision: User selects option → continues to next tree node or another custom step
+- Custom action: User clicks Continue → returns to tree flow (next_node_id of original tree node)
+- Custom solution: User clicks Complete → ends session
+
+#### B.12: Update Session Export
+**File:** `frontend/src/components/session/ExportPreviewModal.tsx` or `backend/app/api/endpoints/sessions.py`

 Ensure custom steps are included in export with clear marking:

@@ -448,7 +581,7 @@ Ensure custom steps are included in export with clear marking:
 Instructions: ...
 ```

-#### B.12: Update Session Types
+#### B.13: Update Session Types
 **File:** `frontend/src/types/index.ts`

 Add:
@@ -487,26 +620,29 @@ export interface CustomStepDraft {

 | File | Issue | Description |
 |------|-------|-------------|
-| `components/tree-editor/ValidationSummary.tsx` | #1 | Error/warning display |
-| `api/steps.ts` | #10 | Steps API client |
-| `api/stepCategories.ts` | #10 | Categories API client |
-| `types/step.ts` | #10 | Step TypeScript types |
-| `components/step-library/StepCard.tsx` | #10 | Step list item card |
-| `components/step-library/StepDetailModal.tsx` | #10 | Step preview modal |
-| `components/step-library/StepLibraryBrowser.tsx` | #10 | Main browser component |
-| `components/step-library/StepForm.tsx` | #9 | Step creation form |
-| `components/step-library/CustomStepModal.tsx` | #9 | Tabbed modal wrapper |
+| `frontend/components/tree-editor/ValidationSummary.tsx` | #1 | Error/warning display |
+| `frontend/api/steps.ts` | #10 | Steps API client |
+| `frontend/api/stepCategories.ts` | #10 | Categories API client |
+| `frontend/types/step.ts` | #10 | Step TypeScript types |
+| `frontend/components/step-library/StepCard.tsx` | #10 | Step list item card |
+| `frontend/components/step-library/StepDetailModal.tsx` | #10 | Step preview modal |
+| `frontend/components/step-library/StepLibraryBrowser.tsx` | #10 | Main browser component |
+| `frontend/components/step-library/StepForm.tsx` | #9 | Step creation form |
+| `frontend/components/step-library/CustomStepModal.tsx` | #9 | Tabbed modal wrapper |
+| `backend/alembic/versions/XXXX_add_custom_steps.py` | #8 | Add custom_steps to sessions |

-### Modified Files (5)
+### Modified Files (6)

 | File | Issue | Changes |
 |------|-------|---------|
-| `store/treeEditorStore.ts` | #1 | Add circular reference detection |
-| `pages/TreeEditorPage.tsx` | #1 | Integrate validation UI |
-| `components/tree-editor/NodeList.tsx` | #1 | Node error indicators |
-| `pages/TreeNavigationPage.tsx` | #8 | Add custom step button + modal |
-| `api/index.ts` | #10 | Export new API clients |
-| `types/index.ts` | #8 | Add CustomStep types |
+| `frontend/store/treeEditorStore.ts` | #1 | Add circular reference detection |
+| `frontend/pages/TreeEditorPage.tsx` | #1 | Integrate validation UI |
+| `frontend/components/tree-editor/NodeList.tsx` | #1 | Node error indicators |
+| `frontend/pages/TreeNavigationPage.tsx` | #8 | Add custom step button + rendering |
+| `frontend/api/index.ts` | #10 | Export new API clients |
+| `frontend/types/index.ts` | #8 | Add CustomStep types |
+| `backend/app/schemas/session.py` | #8 | Add custom_steps field |
+| `backend/app/api/endpoints/sessions.py` | #8 | Handle custom_steps in update |

 ---

@@ -536,8 +672,9 @@ Workstream A                    Workstream B
 ─────────────                   ─────────────
 Testing & polish                B.8 StepForm
                                B.9 CustomStepModal
-                                B.10 TreeNavigationPage integration
-                                B.11-12 Export & types updates
+                                B.10 Backend custom steps support (migration + schemas)
+                                B.11 TreeNavigationPage integration
+                                B.12-13 Export & types updates
 ```

 ---
@@ -578,13 +715,54 @@ Testing & polish                B.8 StepForm

 ---

-## Questions to Resolve Before Starting
+## Design Decisions (Resolved)

-1. **Custom steps persistence**: Should custom steps be saved to the session in the database, or only exist in frontend state until export?
-   - *Recommendation*: Save to session.decisions with a `is_custom_step: true` flag
+### 1. Custom Steps Persistence ✅
+**Decision:** Separate `custom_steps` JSONB field in sessions table

-2. **Step library page**: Should there be a standalone `/steps` page for browsing the library outside of navigation?
-   - *Recommendation*: Yes, add this as a future enhancement after Issue #10
+**Rationale:**
+- Clean separation of concerns (decisions track actions taken, custom_steps store step content)
+- Enables session resuming with custom steps intact
+- Easier to query and export custom steps separately
+- Follows normalized data structure patterns

-3. **Rate limiting**: Should step creation have rate limiting to prevent spam?
-   - *Recommendation*: Backend concern, out of scope for this plan
+**Implementation:** Task B.10 adds migration + schema updates
+
+### 2. Custom Step Navigation Flow ✅
+**Decision:** Full step type support (decision/action/solution)
+
+**Rationale:**
+- Real-world troubleshooting requires branching after custom actions
+- Example: "Try clearing cache" (action) → "Did it work?" (decision)
+- Consistent with tree structure mental model
+- Maximum flexibility for engineers
+
+**Implementation:** Task B.11 renders custom steps based on `step_type`
+
+### 3. Validation Warning UX ✅
+**Decision:** Inline dismissible warnings, no confirmation modal
+
+**Rationale:**
+- Warnings are informational, not critical
+- Orphan nodes might be intentional (work in progress)
+- Trust users to manage their own tree quality
+- Simpler UX, fewer clicks
+- Draft feature (Issue #25) handles "save incomplete work" use case
+
+**Implementation:** Task A.2-A.3 show warnings in ValidationSummary, don't block save
+
+---
+
+## Future Enhancements (Out of Scope)
+
+1. **Draft Trees Feature** - Documented in Issue #25
+   - Save incomplete trees without validation errors blocking
+   - Planned for Phase 3 after Step Library is complete
+
+2. **Standalone Step Library Page** - `/steps` route
+   - Browse/manage personal step library outside of active sessions
+   - Add after Issue #10 is complete and usage patterns are understood
+
+3. **Rate Limiting** - Backend concern
+   - Prevent step library spam
+   - Add if abuse is observed in production