|
| 1 | +# Phase 21D Plan — Frontend Split Final Closeout & InspectorRouter State Resolution |
| 2 | + |
| 3 | +## Status |
| 4 | + |
| 5 | +Planned — Phase 21A/21B/21C complete. No existing 21D plan file. |
| 6 | + |
| 7 | +## Phase Objective |
| 8 | + |
| 9 | +Resolve the remaining frontend split closeout issues: remove fake/stub annotation and pipeline state from `InspectorRouter`, finalize Phase 21 with honest section-owned inspector rendering, and update all planning artifacts. |
| 10 | + |
| 11 | +## What Phase 21B Established (Do Not Regress) |
| 12 | + |
| 13 | +- `useRuntimeStatus` is called exactly once in `App.tsx` |
| 14 | +- `ReadinessStrip` receives `runtimeReadiness` as a prop from `App.tsx` |
| 15 | +- `runtimeState.health` derives from `runtimeStatus.readiness` (backend truth) |
| 16 | +- `runtime-selectors.ts` eligibility functions are unchanged |
| 17 | +- `runtime-selectors.test.ts` regression tests pass |
| 18 | + |
| 19 | +## What Phase 21C Established (Do Not Regress) |
| 20 | + |
| 21 | +- `JobsPanel` owns `threshold` locally via `useState(62)` |
| 22 | +- `AnnotationEnginePanel` owns all annotation state internally |
| 23 | +- `PipelinePanel` owns pipeline definition/validation/selected node internally |
| 24 | +- `OverviewPanel` shows neutral pipeline text (no fake validation claim) |
| 25 | +- `JobsPanel` and `TimelineReplayPanel` use `seededGroundTruth` via `useMemo` |
| 26 | +- `AppRoutes` has 17 props |
| 27 | + |
| 28 | +## Current Problem |
| 29 | + |
| 30 | +`AppRoutes` passes 8 fake/stub props to `InspectorRouter`: |
| 31 | + |
| 32 | +```tsx |
| 33 | +annotations={[]} |
| 34 | +selectedAnnotation="" |
| 35 | +setSelectedAnnotation={() => {}} |
| 36 | +threshold={62} |
| 37 | +setThreshold={() => {}} |
| 38 | +pipelineSelectedNodeId="detector" |
| 39 | +pipelineDefinition={demoSnapshot.pipeline} |
| 40 | +pipelineValidation={validatePipelineDefinition(demoSnapshot.pipeline)} |
| 41 | +``` |
| 42 | + |
| 43 | +`InspectorRouter` uses these to render `AnnotationInspector` and `PipelineInspector` in the global right rail. However: |
| 44 | + |
| 45 | +1. **PipelinePanel already renders `PipelineInspector` internally** at its right column (`grid-cols-[minmax(0,1.35fr)_320px]`), with real state derived from its own local `definition`, `validation`, `selectedNodeId`. |
| 46 | +2. **AnnotationEnginePanel already owns annotation state internally** and has its own embedded annotation inspector. |
| 47 | +3. Rendering a second inspector in the global rail with fake/stale state is dishonest and creates confusion. |
| 48 | + |
| 49 | +## Architecture Decision |
| 50 | + |
| 51 | +**Chosen option: A1 — Honest embedded-notice panels for annotate/pipeline sections in InspectorRouter.** |
| 52 | + |
| 53 | +For `annotate` and `pipeline` sections, the global `InspectorRouter` will render a non-interactive notice stating that the inspector is embedded in the workspace. For `media`, `datasets`, `jobs`, it continues rendering real data. |
| 54 | + |
| 55 | +This is preferred because: |
| 56 | +- `PipelinePanel` and `AnnotationEnginePanel` already own their inspector state locally |
| 57 | +- The global right rail inspector for those sections was always fake/secondary |
| 58 | +- Option A1 preserves the prop-surface gains from Phase 21C (no lifting state back to App root) |
| 59 | +- Option B (extracting route containers) is heavier and would increase prop surface |
| 60 | + |
| 61 | +## Scope |
| 62 | + |
| 63 | +### In Scope |
| 64 | +- Refactor `InspectorRouterProps` to remove: `annotations`, `selectedAnnotation`, `setSelectedAnnotation`, `threshold`, `setThreshold`, `pipelineSelectedNodeId`, `pipelineDefinition`, `pipelineValidation` |
| 65 | +- Update `InspectorRouter` to render honest embedded-notice panels for `annotate` and `pipeline` |
| 66 | +- Update `AppRoutes.tsx` to remove fake prop passings and unused imports (`validatePipelineDefinition`, `PipelineValidationResult`, `demoSnapshot.pipeline`) |
| 67 | +- Verify no no-op setters are passed to visible inspector controls |
| 68 | +- Verify `PipelinePanel` and `AnnotationEnginePanel` own their inspectors independently (already true — confirm) |
| 69 | +- Update `inspector/index.ts` barrel export (may need adjustment) |
| 70 | +- Update `inspector.types.ts` — remove `AnnotationInspectorData` and `PipelineInspectorData` if only used by the removed props |
| 71 | +- Circular dependency audit of app/feature/shared imports |
| 72 | +- Preserve `seededGroundTruth` for Jobs/Timeline |
| 73 | +- Preserve `runtimeReadiness` flow |
| 74 | + |
| 75 | +### Out of Scope |
| 76 | +- No backend changes |
| 77 | +- No Prisma/schema changes |
| 78 | +- No new product feature |
| 79 | +- No UI redesign |
| 80 | +- No lifting annotation/pipeline state back to App.tsx |
| 81 | +- No Redux/Zustand/React Query |
| 82 | +- No creating new feature route containers (Option B) |
| 83 | +- No visual polish unless a layout regression is caused |
| 84 | +- No changing `useRuntimeStatus` call site |
| 85 | +- No changing `runtime-selectors` behavior |
| 86 | + |
| 87 | +## Implementation Steps |
| 88 | + |
| 89 | +### Step 1 — Refactor InspectorRouterProps |
| 90 | + |
| 91 | +Reduce `InspectorRouterProps` from: |
| 92 | + |
| 93 | +```typescript |
| 94 | +type InspectorRouterProps = { |
| 95 | + active: SectionId; |
| 96 | + annotations: AnnotationSummary[]; |
| 97 | + selectedAnnotation: string; |
| 98 | + setSelectedAnnotation: (id: string) => void; |
| 99 | + threshold: number; |
| 100 | + setThreshold: (v: number) => void; |
| 101 | + job: JobUiState; |
| 102 | + predictions: PredictionSummary[]; |
| 103 | + pipelineSelectedNodeId: string; |
| 104 | + pipelineDefinition: PipelineDefinition; |
| 105 | + pipelineValidation: PipelineValidationResult; |
| 106 | + mediaInspectorData: MediaInspectorData; |
| 107 | + datasetInspectorData: DatasetInspectorData; |
| 108 | + projectName: string; |
| 109 | +}; |
| 110 | +``` |
| 111 | + |
| 112 | +to: |
| 113 | + |
| 114 | +```typescript |
| 115 | +type InspectorRouterProps = { |
| 116 | + active: SectionId; |
| 117 | + job: JobUiState; |
| 118 | + predictions: PredictionSummary[]; |
| 119 | + mediaInspectorData: MediaInspectorData; |
| 120 | + datasetInspectorData: DatasetInspectorData; |
| 121 | + projectName: string; |
| 122 | +}; |
| 123 | +``` |
| 124 | + |
| 125 | +### Step 2 — Update InspectorRouter |
| 126 | + |
| 127 | +For `active === 'annotate'`: |
| 128 | +- Render `InspectorShell` with honest text: "Annotation inspector is embedded in the workspace panel." |
| 129 | + |
| 130 | +For `active === 'pipeline'`: |
| 131 | +- Render `InspectorShell` with honest text: "Pipeline node inspector is embedded in the Pipeline workspace." |
| 132 | + |
| 133 | +For `active === 'overview'`, `active === 'timeline'`, `active === 'diff'`: |
| 134 | +- Keep existing fallback (project name + job status) |
| 135 | + |
| 136 | +For `active === 'media'`, `active === 'datasets'`, `active === 'jobs'`: |
| 137 | +- Keep existing real inspector rendering (unchanged) |
| 138 | + |
| 139 | +### Step 3 — Update AppRoutes.tsx |
| 140 | + |
| 141 | +Remove all fake prop passings to `InspectorRouter`: |
| 142 | +- Remove `annotations={[]}` |
| 143 | +- Remove `selectedAnnotation=""` |
| 144 | +- Remove `setSelectedAnnotation={() => {}}` |
| 145 | +- Remove `threshold={62}` |
| 146 | +- Remove `setThreshold={() => {}}` |
| 147 | +- Remove `pipelineSelectedNodeId="detector"` |
| 148 | +- Remove `pipelineDefinition={demoSnapshot.pipeline}` |
| 149 | +- Remove `pipelineValidation={...}` |
| 150 | + |
| 151 | +Remove unused imports: |
| 152 | +- `validatePipelineDefinition` from `@visionflow/contracts` |
| 153 | +- `PipelineValidationResult` from `@visionflow/contracts` |
| 154 | +- `AnnotationSummary`, `PipelineDefinition`, `PipelineNode` (if only used for fake props) |
| 155 | +- `demoSnapshot` (if only used for `demoSnapshot.pipeline` fake prop — check if `demoSnapshot.project.name` is still needed) |
| 156 | + |
| 157 | +Note: `demoSnapshot.project.name` is still passed to `projectName`. Keep `demoSnapshot` import if needed for that. Check if `demoSnapshot` is used elsewhere in AppRoutes. |
| 158 | + |
| 159 | +Remove `createSeedAnnotationSummaries` import? No — it is still used for `seededGroundTruth` in Jobs/Timeline. |
| 160 | + |
| 161 | +### Step 4 — Update inspector/index.ts |
| 162 | + |
| 163 | +Check if `InspectorRouterProps` is still exported from barrel. If the type shape changed, update consumers of `InspectorRouterProps` if any exist outside InspectorRouter. |
| 164 | + |
| 165 | +### Step 5 — Update inspector.types.ts |
| 166 | + |
| 167 | +Remove `AnnotationInspectorData` and `PipelineInspectorData` from `inspector.types.ts` if they are no longer referenced anywhere (they are only used by the now-removed props). |
| 168 | + |
| 169 | +### Step 6 — Audit imports |
| 170 | + |
| 171 | +Grep for the removed patterns in AppRoutes to confirm they're gone: |
| 172 | +- `annotations={[]}` |
| 173 | +- `setSelectedAnnotation` |
| 174 | +- `setThreshold` |
| 175 | +- `pipelineDefinition={demoSnapshot.pipeline}` |
| 176 | +- `pipelineValidation={validatePipelineDefinition` |
| 177 | + |
| 178 | +### Step 7 — Verify PipelinePanel self-containment |
| 179 | + |
| 180 | +Confirm `PipelinePanel`: |
| 181 | +- Renders `PipelineInspector` internally (confirmed: line ~346-361) |
| 182 | +- Does not depend on `InspectorRouter` for pipeline inspector state |
| 183 | +- Uses its own local `definition`, `validation`, `selectedNodeId` |
| 184 | + |
| 185 | +Confirm `AnnotationEnginePanel`: |
| 186 | +- Owns annotation state internally (confirmed from Phase 21C) |
| 187 | +- Does not depend on `InspectorRouter` for annotation state |
| 188 | + |
| 189 | +### Step 8 — Circular dependency audit |
| 190 | + |
| 191 | +Check imports: |
| 192 | +- `app/` imports from `features/inspector/` — `InspectorRouter` only (AppRoutes.tsx) |
| 193 | +- `features/inspector/` imports from `@visionflow/contracts` — no app/ or feature-local imports |
| 194 | +- `features/inspector/` does NOT import from `app/` |
| 195 | +- `shared/` does NOT import from `app/` or `features/inspector/` |
| 196 | +- `app/` does NOT import from `features/annotations/` internals beyond `AnnotationEnginePanel` |
| 197 | +- `app/` does NOT import from `features/pipeline/` internals (PipelinePanel is in app/) |
| 198 | + |
| 199 | +## Non-Goals |
| 200 | + |
| 201 | +- No lifting annotation/pipeline mutable state to App.tsx |
| 202 | +- No creating new route container files |
| 203 | +- No changing runtime truth flow |
| 204 | +- No visual redesign of existing panels |
| 205 | + |
| 206 | +## File Map |
| 207 | + |
| 208 | +### Files Changed |
| 209 | + |
| 210 | +| File | Change | |
| 211 | +|---|---| |
| 212 | +| `apps/web/src/features/inspector/InspectorRouter.tsx` | Remove 8 fake props; render honest embedded-notice for annotate/pipeline | |
| 213 | +| `apps/web/src/app/AppRoutes.tsx` | Remove fake prop passings; remove unused imports | |
| 214 | +| `apps/web/src/features/inspector/index.ts` | Update `InspectorRouterProps` type export (if shape changed) | |
| 215 | +| `apps/web/src/features/inspector/inspector.types.ts` | Remove `AnnotationInspectorData` and `PipelineInspectorData` (if unreferenced) | |
| 216 | + |
| 217 | +### Files Unchanged (guarded) |
| 218 | + |
| 219 | +- `App.tsx` — no changes needed |
| 220 | +- `PipelinePanel.tsx` — no changes needed (already self-contained) |
| 221 | +- `AnnotationEnginePanel` — no changes needed (already self-contained) |
| 222 | +- `JobsPanel.tsx` — no changes needed |
| 223 | +- `TimelineReplayPanel` — no changes needed |
| 224 | +- `ReadinessStrip.tsx` — no changes needed |
| 225 | +- `useRuntimeStatus.ts` — no changes needed |
| 226 | +- `runtime-selectors.ts` — no changes needed |
| 227 | +- `runtime-selectors.test.ts` — no changes needed |
| 228 | + |
| 229 | +## Verification Plan |
| 230 | + |
| 231 | +1. `pnpm --filter @visionflow/web typecheck` — must pass |
| 232 | +2. `pnpm --filter @visionflow/web test` — all tests pass |
| 233 | +3. `pnpm --filter @visionflow/web lint` — must pass |
| 234 | +4. `pnpm --filter @visionflow/web build` — must pass |
| 235 | +5. Runtime truth grep checks: |
| 236 | + - `grep "useRuntimeStatus" apps/web/src/App.tsx` — only one call site |
| 237 | + - `grep "runtimeReadiness" apps/web/src/` — correct flow |
| 238 | +6. Fake-state grep checks: |
| 239 | + - `grep "annotations={}" apps/web/src/` — zero results |
| 240 | + - `grep "setSelectedAnnotation={() => {}}" apps/web/src/` — zero results |
| 241 | + - `grep "setThreshold={() => {}}" apps/web/src/` — zero results |
| 242 | + - `grep "pipelineDefinition={demoSnapshot" apps/web/src/` — zero results |
| 243 | + - `grep "pipelineValidation={validatePipelineDefinition" apps/web/src/` — zero results |
| 244 | +7. Runtime truth invariants unchanged: |
| 245 | + - `useRuntimeStatus` called only in App.tsx |
| 246 | + - `runtimeState.health` derives from backend truth |
| 247 | + - `runtime-selectors.test.ts` passes |
| 248 | +8. Phase 21C preservation: |
| 249 | + - JobsPanel still uses `seededGroundTruth` |
| 250 | + - TimelineReplayPanel still uses `seededGroundTruth` |
| 251 | + - OverviewPanel still shows neutral pipeline text |
| 252 | + - AppRoutes prop count does not increase |
| 253 | +9. Browser smoke (if stack running): |
| 254 | + - Annotate page shows annotation inspector embedded in canvas panel |
| 255 | + - Pipeline page shows pipeline inspector embedded in pipeline panel |
| 256 | + - No fake annotation state in the global inspector rail |
| 257 | + - No fake pipeline state in the global inspector rail |
| 258 | + - Media/Datasets/Jobs inspectors still show real data |
| 259 | + |
| 260 | +## Success Criteria |
| 261 | + |
| 262 | +1. No fake annotation state is passed to InspectorRouter |
| 263 | +2. No fake pipeline state is passed to InspectorRouter |
| 264 | +3. No no-op setter is passed to a visible inspector control |
| 265 | +4. Annotation/pipeline inspector ownership is explicit and documented |
| 266 | +5. Global inspector rail renders honest embedded-notice for annotate/pipeline |
| 267 | +6. `useRuntimeStatus` remains single-call at App root |
| 268 | +7. `runtimeState.health` remains backend-derived |
| 269 | +8. `seededGroundTruth` preserved for Jobs and Timeline |
| 270 | +9. Overview neutral pipeline text preserved |
| 271 | +10. AppRoutes prop surface does not balloon |
| 272 | +11. Phase 21A/21B/21C invariants all preserved |
| 273 | +12. Typecheck/test/lint/build all pass |
| 274 | +13. All planning artifacts updated (STATE, ROADMAP, MILESTONES, 21D-SUMMARY, 21D-REVIEW) |
0 commit comments