apps/ui/src/components/layout/project-switcher/components/project-switcher-item.tsx (1)

40-47: Harden test-id slugging to avoid selector escapes and collisions.

Whitespace-only replacement leaves special characters intact and can produce duplicate IDs for same-named projects. Consider a stricter slug and a fallback to project.id when empty/duplicated.

♻️ Suggested slugging refinement

-  // Create a sanitized project name for test ID (lowercase, hyphens instead of spaces)
-  const sanitizedName = project.name.toLowerCase().replace(/\s+/g, '-');
+  // Create a sanitized project name for test ID (lowercase, URL-safe slug)
+  const sanitizedName =
+    project.name
+      .toLowerCase()
+      .trim()
+      .replace(/[^a-z0-9]+/g, '-')
+      .replace(/^-+|-+$/g, '') || `project-${project.id}`;

apps/ui/src/components/views/spec-view/types.ts (1)

1-2: Consider renaming to avoid collision with the SpecViewMode component.

The type SpecViewMode shares a name with the SpecViewMode component exported from ./spec-view-mode.tsx. While TypeScript distinguishes between type and value imports, this can cause confusion when importing both:

import { SpecViewMode } from './spec-view-mode'; // component
import type { SpecViewMode } from '../types';    // type - collision

Consider renaming the type to SpecViewModeType or ViewMode to improve clarity.

apps/ui/src/components/views/spec-view/components/spec-mode-tabs.tsx (2)

29-44: Consider adding accessible labels for icon-only tabs on mobile.

On small screens (sm:hidden text), tabs display only icons. Screen reader users may not understand what each tab represents. Consider adding aria-label attributes to the triggers.

♿ Suggested improvement for accessibility

         <TabsTrigger
           value="view"
           disabled={disabled || !isParseValid}
           title={!isParseValid ? 'Fix XML errors to enable View mode' : 'View formatted spec'}
+          aria-label="View"
         >
           <Eye className="w-4 h-4" />
           <span className="hidden sm:inline">View</span>
         </TabsTrigger>
         <TabsTrigger
           value="edit"
           disabled={disabled || !isParseValid}
           title={!isParseValid ? 'Fix XML errors to enable Edit mode' : 'Edit spec with form'}
+          aria-label="Edit"
         >
           <Edit3 className="w-4 h-4" />
           <span className="hidden sm:inline">Edit</span>
         </TabsTrigger>
-        <TabsTrigger value="source" disabled={disabled} title="Edit raw XML source">
+        <TabsTrigger value="source" disabled={disabled} title="Edit raw XML source" aria-label="Source">
           <Code className="w-4 h-4" />
           <span className="hidden sm:inline">Source</span>
         </TabsTrigger>

---

18-20: Type assertion is acceptable but could be more defensive.

The cast value as SpecViewMode is safe given the hardcoded tab values, but for robustness you could validate before casting. This is optional given the controlled context.

apps/ui/src/components/views/spec-view/components/edit-mode/features-section.tsx (2)

156-178: Fallback key pattern may cause reconciliation issues.

The fallback fallback-${idx} at line 158 could cause React reconciliation issues if _locationIds becomes out of sync with file_locations. Since featureToInternal always generates _locationIds, this fallback should rarely trigger, but consider logging a warning in development if it does to catch bugs early.

---

97-98: Consider future drag-and-drop implementation.

The GripVertical icon suggests drag-and-drop reordering capability. If this is planned for later, consider adding a comment or tracking it as a TODO.

apps/ui/src/components/views/spec-view/components/spec-edit-mode.tsx (1)

24-46: JSON.stringify comparison has key ordering limitations.

Using JSON.stringify for deep comparison (lines 25, 32, 42) works for most cases but is sensitive to key ordering. If the spec object's keys could be reordered between renders (e.g., from different serialization sources), this could cause unnecessary resets. In practice this is usually fine for structured data like SpecOutput, but worth noting.

libs/spec-parser/src/validate.ts (1)

130-143: Consider adding closing tag check for consistency.

The root element check only verifies the opening tag while other element checks use regex that implicitly require both tags. For a more robust quick check, consider also verifying the closing tag exists:

💡 Optional improvement

   // Check for essential elements
-  const hasRoot = xmlContent.includes('<project_specification>');
+  const hasRoot =
+    xmlContent.includes('<project_specification>') &&
+    xmlContent.includes('</project_specification>');

apps/ui/src/components/views/spec-view/hooks/use-spec-parser.ts (1)

41-49: Consider whether pre-check masks detailed errors.

The isValidSpecXml pre-check returns a generic "Invalid XML structure - missing required elements" error. When this fails, the more detailed errors from xmlToSpec are bypassed. If users would benefit from specific field-level error messages, consider letting xmlToSpec handle all validation, or include which elements are missing in the quick-check error.

This is a trade-off between performance (fast-fail) and error specificity. The current approach is reasonable for obviously malformed input.

apps/ui/src/components/views/spec-view/components/edit-mode/array-field-editor.tsx (1)

40-48: External sync regenerates all IDs, causing full remount.

When values changes externally, all items receive new UUIDs (Line 47), causing React to remount every item. This works correctly but may cause minor UX issues if the parent inadvertently changes the values array reference with identical content.

Consider a shallow comparison to avoid unnecessary ID regeneration:

💡 Optional: Avoid remount when content hasn't changed

   useEffect(() => {
     if (isInternalChange.current) {
       isInternalChange.current = false;
       return;
     }
 
-    // External change - rebuild items with new IDs
-    setItems(values.map((value) => ({ id: generateId(), value })));
+    // External change - only rebuild if content actually changed
+    const currentValues = items.map((item) => item.value);
+    const valuesMatch =
+      values.length === currentValues.length &&
+      values.every((v, i) => v === currentValues[i]);
+    if (!valuesMatch) {
+      setItems(values.map((value) => ({ id: generateId(), value })));
+    }
   }, [values]);

apps/ui/src/components/views/spec-view/components/edit-mode/features-section.tsx (1)

156-157: Fallback key using index may cause instability.

The fallback-${idx} key is used when _locationIds is undefined or shorter than file_locations. Since featureToInternal always generates _locationIds from the locations array, this fallback should rarely trigger. However, if synchronization ever fails (e.g., edge case in external data), the index-based fallback can cause React reconciliation issues when locations are added/removed.

Consider making this more defensive by ensuring _locationIds is always in sync, or logging a warning when the fallback is used to catch potential bugs early.

apps/ui/src/components/views/spec-view.tsx (1)

198-216: The disabled={isProcessing} prop on line 207 is effectively unused.

The parent conditional on line 201 (!isProcessing &&) ensures the SpecModeTabs component only renders when isProcessing is false, so disabled={isProcessing} will always evaluate to false.

Consider removing the redundant prop or, if the intent is to show disabled tabs during processing, render the tabs unconditionally and rely solely on the disabled prop.

Option 1: Remove the redundant prop

             <SpecModeTabs
               mode={mode}
               onModeChange={handleModeChange}
               isParseValid={isParseValid}
-              disabled={isProcessing}
             />

Option 2: Show disabled tabs during processing (if intended)

-        {!isProcessing && (
           <div className="flex items-center justify-center px-4 py-2 border-b border-border bg-muted/30 relative">
             <SpecModeTabs
               mode={mode}
               onModeChange={handleModeChange}
               isParseValid={isParseValid}
               disabled={isProcessing}
             />
             ...
           </div>
-        )}

apps/ui/src/components/views/spec-view/components/edit-mode/roadmap-section.tsx (1)

1-16: Imports look good; Map shadowing issue resolved.

The Map icon is correctly aliased to MapIcon (line 15) addressing the previous review concern. The @automaker/spec-parser import follows the coding guidelines.

Minor nit: the two lucide-react imports (lines 1 and 15) could be consolidated into one.