From 972671595664272b092e5310188b69c7a490aab4 Mon Sep 17 00:00:00 2001 From: rdfitted Date: Thu, 22 Jan 2026 15:25:52 -1000 Subject: [PATCH 1/5] feat: add master checkbox for bulk selection in manual cleanup - Add tri-state master checkbox to DuplicateGroupsPreview header - Use useMemo for allSelected and isIndeterminate calculations - Use useRef + useEffect to sync indeterminate DOM property - Add accessibility support (aria-label, keyboard navigation) - Replace old text buttons with master checkbox UI - Match styling to existing individual checkboxes - Hide master checkbox when no groups present Resolves #49 Co-Authored-By: Claude Opus 4.5 --- .ai-docs/learnings.jsonl | 1 + .../migration/DuplicateGroupsPreview.tsx | 83 +++++++++++++------ 2 files changed, 60 insertions(+), 24 deletions(-) diff --git a/.ai-docs/learnings.jsonl b/.ai-docs/learnings.jsonl index 0ff5ed9..58d46e5 100644 --- a/.ai-docs/learnings.jsonl +++ b/.ai-docs/learnings.jsonl @@ -2,3 +2,4 @@ {"date":"2026-01-20","session":"20260120-193522-resolve-issue-37","task":"Add source filters to duplicate cleanup feature","outcome":"success","keywords":["filter","cleanup","duplicate","validation","ui","backend","podio"],"insight":"Reusing existing filter infrastructure (ItemMigrationFilters, convertFilters) accelerates feature development. Reviewers caught 8 important issues including missing date validation, filter state handling bugs, and backwards compatibility gaps. Sequential worker pattern (backend→frontend→coherence) ensures clean layer separation. Filter persistence in job metadata enables reproducibility.","files_touched":["lib/migration/cleanup/types.ts","lib/migration/cleanup/service.ts","lib/migration/cleanup/executor.ts","app/components/migration/CleanupPanel.tsx","app/api/migration/cleanup/route.ts"]} {"date":"2026-01-21","session":"20260121-145015-resolve-issue-45","task":"Persist cleanup job state across page refresh","outcome":"success","keywords":["localStorage","persistence","SSR","TTL","auto-reconnect","job-history","cleanup","pagination","AbortController","race-condition"],"insight":"localStorage persistence requires SSR safety (typeof window check), QuotaExceededError/SecurityError handling, and schema validation for JSON parsing. Auto-reconnect logic needs AbortController to prevent race conditions on mount. useMemo is better than useEffect for derived state like pagination. Following existing patterns (MigrationContext for storage, ItemMigrationPanel for job history UI) accelerates development significantly.","files_touched":["app/hooks/useCleanup.ts","app/components/migration/CleanupPanel.tsx","lib/migration/cleanup/types.ts","lib/migration/cleanup/service.ts"]} {"date":"2026-01-22","session":"20260122-fix-comment","task":"Fix race conditions and type duplication from code review","outcome":"success","keywords":["AbortController","race-condition","stale-closure","useEffect","dependency-array","interface","extends","Partial","code-review","gemini-code-assist"],"insight":"Code review bots (gemini-code-assist) catch valid race condition issues. Three patterns validated: (1) async useEffect needs AbortController + isMounted guard, (2) props in effect body must be in deps array - dont suppress eslint, (3) extend interfaces with Partial instead of duplicating fields. Following existing patterns in same file accelerates fixes.","files_touched":["app/components/migration/CleanupPanel.tsx","app/hooks/useCleanup.ts","lib/migration/cleanup/types.ts"]} +{"date":"2026-01-22","session":"20260122-144008-resolve-issue-49","task":"Resolve issue #49: Add master checkbox for bulk selection","outcome":"success","keywords":["checkbox","tri-state","useMemo","useRef","accessibility","react-hooks","indeterminate","bulk-selection","hive","multi-agent"],"insight":"Session guidelines codification upfront prevents implementation issues. Sequential workers (Opus->Gemini->GLM->Codex) execute cleanly when each follows shared guidelines. Tester phase catches edge cases missed by code reviewers (empty groups visibility). Key pattern: strong upfront guidelines + sequential layered approach + testing = clean implementation with zero reviewer issues.","files_touched":["app/components/migration/DuplicateGroupsPreview.tsx"]} diff --git a/app/components/migration/DuplicateGroupsPreview.tsx b/app/components/migration/DuplicateGroupsPreview.tsx index 35da84b..ec970fb 100644 --- a/app/components/migration/DuplicateGroupsPreview.tsx +++ b/app/components/migration/DuplicateGroupsPreview.tsx @@ -5,8 +5,8 @@ 'use client'; -import React, { useState } from 'react'; -import { DuplicateGroup, DuplicateItem, CleanupMode } from '@/lib/migration/cleanup/types'; +import React, { useState, useMemo, useRef, useEffect } from 'react'; +import { DuplicateGroup, CleanupMode } from '@/lib/migration/cleanup/types'; export interface DuplicateGroupsPreviewProps { groups: DuplicateGroup[]; @@ -28,6 +28,36 @@ export function DuplicateGroupsPreview({ const [expandedGroups, setExpandedGroups] = useState>(new Set()); const [selectedGroups, setSelectedGroups] = useState>(new Set()); + // Master checkbox ref for setting indeterminate property (DOM-only, not a React attribute) + const masterCheckboxRef = useRef(null); + + // Derived state for master checkbox tri-state logic + const allSelected = useMemo( + () => groups.length > 0 && selectedGroups.size === groups.length, + [groups.length, selectedGroups.size] + ); + + const isIndeterminate = useMemo( + () => selectedGroups.size > 0 && selectedGroups.size < groups.length, + [selectedGroups.size, groups.length] + ); + + // Sync indeterminate property to DOM (not available as React attribute) + useEffect(() => { + if (masterCheckboxRef.current) { + masterCheckboxRef.current.indeterminate = isIndeterminate; + } + }, [isIndeterminate]); + + const isManualInteractive = mode === 'manual' && !dryRun; + + let masterCheckboxAriaLabel = 'Select all groups'; + if (allSelected) { + masterCheckboxAriaLabel = 'Deselect all groups'; + } else if (isIndeterminate) { + masterCheckboxAriaLabel = 'Select all groups (some selected)'; + } + const toggleGroup = (index: number) => { const newExpanded = new Set(expandedGroups); if (newExpanded.has(index)) { @@ -56,6 +86,15 @@ export function DuplicateGroupsPreview({ setSelectedGroups(new Set()); }; + // Master checkbox toggle: if all selected, deselect all; otherwise select all + const handleMasterToggle = () => { + if (allSelected) { + deselectAll(); + } else { + selectAll(); + } + }; + const handleExecute = () => { if (!onApproveAndExecute) return; @@ -74,7 +113,7 @@ export function DuplicateGroupsPreview({ .filter((_, idx) => selectedGroups.has(idx)) .reduce((sum, g) => sum + (g.deleteItemIds?.length || g.items.length - 1), 0); - const canExecute = mode === 'manual' && !dryRun && selectedGroups.size > 0; + const canExecute = isManualInteractive && selectedGroups.size > 0; return (
@@ -90,21 +129,20 @@ export function DuplicateGroupsPreview({

- {mode === 'manual' && !dryRun && ( -
- - -
+ + )} @@ -114,9 +152,6 @@ export function DuplicateGroupsPreview({ const isExpanded = expandedGroups.has(groupIdx); const isSelected = selectedGroups.has(groupIdx); const itemsToDelete = group.deleteItemIds?.length || group.items.length - 1; - const keepItem = group.keepItemId - ? group.items.find(i => i.itemId === group.keepItemId) - : group.items[0]; // Default to first (oldest) if not specified return (
toggleGroup(groupIdx)} >
- {mode === 'manual' && !dryRun && ( + {isManualInteractive && ( toggleSelection(groupIdx)} onClick={(e) => e.stopPropagation()} - className="h-4 w-4" + className="h-4 w-4 rounded border-gray-300 text-blue-600 focus:ring-blue-500 dark:border-gray-600 dark:bg-gray-700" + aria-label={`Select group with match value ${group.matchValue}`} /> )}
@@ -180,7 +216,6 @@ export function DuplicateGroupsPreview({ const isKeep = group.keepItemId ? item.itemId === group.keepItemId : itemIdx === 0; - const willDelete = !isKeep; return (
{/* Action Buttons */} - {mode === 'manual' && !dryRun && ( + {isManualInteractive && (
From bee953b20e8e324b69714dfb13b4e26a6a35489d Mon Sep 17 00:00:00 2001 From: rdfitted Date: Thu, 22 Jan 2026 15:45:35 -1000 Subject: [PATCH 2/5] docs: curate learnings and update project DNA after PR review - Added learning from resolveprcomments-50 session - Key insight: aria-checked should NOT be on native checkboxes - Multi-agent verification (4 OpenCode models) with tie-breaker works - Regenerated project-dna.md with 6 session learnings - Added accessibility standards patterns to DNA - Updated hot spots and keyword clusters Co-Authored-By: Codex GPT-5.2 --- .ai-docs/learnings.jsonl | 1 + .ai-docs/project-dna.md | 201 ++++++++++++++++++++++++--------------- 2 files changed, 126 insertions(+), 76 deletions(-) diff --git a/.ai-docs/learnings.jsonl b/.ai-docs/learnings.jsonl index 58d46e5..1b5ac02 100644 --- a/.ai-docs/learnings.jsonl +++ b/.ai-docs/learnings.jsonl @@ -3,3 +3,4 @@ {"date":"2026-01-21","session":"20260121-145015-resolve-issue-45","task":"Persist cleanup job state across page refresh","outcome":"success","keywords":["localStorage","persistence","SSR","TTL","auto-reconnect","job-history","cleanup","pagination","AbortController","race-condition"],"insight":"localStorage persistence requires SSR safety (typeof window check), QuotaExceededError/SecurityError handling, and schema validation for JSON parsing. Auto-reconnect logic needs AbortController to prevent race conditions on mount. useMemo is better than useEffect for derived state like pagination. Following existing patterns (MigrationContext for storage, ItemMigrationPanel for job history UI) accelerates development significantly.","files_touched":["app/hooks/useCleanup.ts","app/components/migration/CleanupPanel.tsx","lib/migration/cleanup/types.ts","lib/migration/cleanup/service.ts"]} {"date":"2026-01-22","session":"20260122-fix-comment","task":"Fix race conditions and type duplication from code review","outcome":"success","keywords":["AbortController","race-condition","stale-closure","useEffect","dependency-array","interface","extends","Partial","code-review","gemini-code-assist"],"insight":"Code review bots (gemini-code-assist) catch valid race condition issues. Three patterns validated: (1) async useEffect needs AbortController + isMounted guard, (2) props in effect body must be in deps array - dont suppress eslint, (3) extend interfaces with Partial instead of duplicating fields. Following existing patterns in same file accelerates fixes.","files_touched":["app/components/migration/CleanupPanel.tsx","app/hooks/useCleanup.ts","lib/migration/cleanup/types.ts"]} {"date":"2026-01-22","session":"20260122-144008-resolve-issue-49","task":"Resolve issue #49: Add master checkbox for bulk selection","outcome":"success","keywords":["checkbox","tri-state","useMemo","useRef","accessibility","react-hooks","indeterminate","bulk-selection","hive","multi-agent"],"insight":"Session guidelines codification upfront prevents implementation issues. Sequential workers (Opus->Gemini->GLM->Codex) execute cleanly when each follows shared guidelines. Tester phase catches edge cases missed by code reviewers (empty groups visibility). Key pattern: strong upfront guidelines + sequential layered approach + testing = clean implementation with zero reviewer issues.","files_touched":["app/components/migration/DuplicateGroupsPreview.tsx"]} +{"date":"2026-01-22","session":"resolveprcomments-50","task":"Resolve PR comments","outcome":"success","keywords":["aria-checked","accessibility","native-checkbox","multi-agent-verification","PR-review","W3C","MDN","consensus-logic"],"insight":"Multi-agent verification (4 OpenCode models) with 2-2 tie resolution by Claude orchestrator works well for nuanced accessibility questions. Key finding: aria-checked should NOT be added to native HTML checkboxes - only to ARIA role='checkbox' elements. Native checkboxes expose indeterminate state automatically to assistive tech. Adding aria-checked to native inputs creates inconsistencies. Web standards research (W3C/MDN) essential for tie-breaker decisions.","files_touched":[]} diff --git a/.ai-docs/project-dna.md b/.ai-docs/project-dna.md index 5bdc250..54e4fe5 100644 --- a/.ai-docs/project-dna.md +++ b/.ai-docs/project-dna.md @@ -1,85 +1,134 @@ # Project DNA - Podio Migration Agent -> Curated patterns and insights from development sessions. Last updated: 2026-01-22 +*Last curated: 2026-01-22* +*Based on: 6 session learnings* + +## Core Patterns + +### What Works + +**Multi-Agent Workflows** +- Sequential worker pattern (implementation→coherence→simplification) ensures clean layer separation - Used in 3 sessions +- Parallel reviewers catch complementary issues (8 issues caught in one session) +- Session guidelines codification upfront prevents implementation issues - Zero reviewer issues when done right +- Multi-agent verification (4 models) with tie-breaker consensus works for nuanced decisions + +**Filter Infrastructure** +- Reusing existing filter infrastructure (ItemMigrationFilters, convertFilters) accelerates feature development +- Filter conversion layer (user-friendly → API format) provides clean separation of concerns +- Filter persistence in job metadata enables reproducibility + +**React Hooks Best Practices** +- `useMemo` for derived state (tri-state logic, pagination) - better than `useEffect` +- `useRef` + `useEffect` for DOM-only properties (`indeterminate`) +- Async `useEffect` needs `AbortController` + `isMounted` guard to prevent race conditions +- Props in effect body MUST be in dependency array - never suppress eslint +- Follow existing patterns in same file for consistency + +**State Persistence** +- localStorage requires SSR safety (`typeof window` check), QuotaExceededError/SecurityError handling +- Schema validation for JSON parsing prevents crashes from corrupted storage +- TTL-based expiration prevents stale data accumulation +- Auto-reconnect logic needs AbortController to prevent mount race conditions + +**Accessibility Standards** +- `aria-checked` should NOT be added to native HTML checkboxes - only for ARIA `role="checkbox"` +- Native checkboxes expose `indeterminate` state automatically to assistive tech +- Web standards research (W3C/MDN) essential for accessibility decisions + +**Validation & Error Handling** +- Custom date validation with ISO 8601 support requires comprehensive edge case testing +- Schema validation with Zod accelerates API development + +**TypeScript Patterns** +- Extend interfaces with `Partial` instead of duplicating fields + +### What Doesn't Work + +**eslint-disable for Dependency Arrays** +- Suppressing `react-hooks/exhaustive-deps` with empty deps while using props leads to stale closures +- The "run only on mount" pattern is wrong when props are involved + +**Field Duplication** +- Duplicating interface fields instead of extending with `Partial` creates maintenance burden + +## Hot Spots (Frequently Modified Files) + +| File | Touch Count | Common Reason | +|------|-------------|---------------| +| `app/components/migration/CleanupPanel.tsx` | 3 | Cleanup feature evolution, race condition fixes, persistence | +| `lib/migration/cleanup/types.ts` | 3 | Filter integration, persistence schema, type fixes | +| `lib/migration/cleanup/service.ts` | 3 | Filter support, validation, executor integration | +| `app/hooks/useCleanup.ts` | 2 | Persistence, race condition fixes | +| `lib/migration/items/filter-converter.ts` | 1 | Initial implementation | +| `lib/migration/items/filter-validator.ts` | 1 | Initial implementation | +| `lib/ai/schemas/migration.ts` | 1 | Filter schema additions | +| `lib/ai/tools.ts` | 1 | Filter parameter additions | +| `lib/migration/items/service.ts` | 1 | Filter integration | +| `lib/migration/items/types.ts` | 1 | Filter types | +| `lib/migration/cleanup/executor.ts` | 1 | Filter persistence | +| `app/api/migration/cleanup/route.ts` | 1 | Filter API integration | +| `app/components/migration/DuplicateGroupsPreview.tsx` | 1 | Master checkbox tri-state implementation | + +*Files touched 3+ times indicate high complexity or rapid feature evolution (cleanup system).* + +## Keyword Clusters + +| Cluster | Keywords | Sessions | +|---------|----------|----------| +| **Cleanup System** | cleanup, duplicate, job-history, persistence, localStorage, TTL | 3 | +| **Filter Infrastructure** | filter, validation, date, schema, zod, api | 2 | +| **React Hooks** | useEffect, useMemo, useRef, AbortController, race-condition, dependency-array, stale-closure | 2 | +| **Accessibility** | accessibility, aria-checked, native-checkbox, indeterminate, tri-state | 2 | +| **Multi-Agent Orchestration** | multi-agent, hive, sequential-workers, parallel-reviewers, consensus-logic | 2 | +| **Type Safety** | interface, extends, Partial, TypeScript | 1 | +| **Code Review** | code-review, gemini-code-assist, PR-review, W3C, MDN | 1 | +| **Podio API** | podio, backend, migration | 2 | +| **UI/Frontend** | ui, frontend, component, bulk-selection, checkbox | 2 | + +## Session Insights (Deduplicated) + +1. **Multi-agent hive with sequential workers and parallel reviewers works well for complex features** - Implementation workers focus on their layer, reviewers catch edge cases across all layers +2. **Filter conversion layer between user-friendly format and API format provides clean separation of concerns** - User sees ISO dates, API gets Podio filter keys +3. **Reusing existing infrastructure accelerates development** - ItemMigrationFilters reused for cleanup feature, MigrationContext pattern reused for cleanup state +4. **Custom validation requires comprehensive edge case testing** - ISO 8601 date formats, empty filters, malformed JSON +5. **localStorage persistence requires defensive coding** - SSR checks, error handling for quota/security, schema validation +6. **useMemo is better than useEffect for derived state** - Pagination, tri-state logic - no async, just compute +7. **Async useEffect needs AbortController + isMounted guard** - Prevents race conditions on mount/unmount +8. **Props in effect body must be in deps array** - Stale closures are the enemy, never suppress eslint +9. **Extend interfaces with Partial instead of duplicating fields** - Single source of truth +10. **Session guidelines codification upfront prevents implementation issues** - Strong guidelines + sequential workers + testing = zero reviewer issues +11. **Multi-agent verification with tie-breaker consensus works for nuanced questions** - 4 models vote, Claude orchestrator resolves 2-2 ties with research +12. **aria-checked is wrong for native checkboxes** - Native elements expose state automatically, adding ARIA creates inconsistencies +13. **Web standards research essential for accessibility decisions** - W3C/MDN authoritative sources resolve conflicting AI opinions +14. **Code review bots catch valid issues** - gemini-code-assist found race conditions and type duplication +15. **Following existing patterns in same file accelerates fixes** - If one effect uses AbortController, all should -## Patterns That Work - -### Multi-Agent Hive Architecture -- Sequential worker pattern (backend→frontend→coherence) ensures clean layer separation (learned: 20260120-142840, 20260120-193522) -- Parallel reviewers catch edge cases that implementation workers miss (learned: 20260120-193522) -- Worker specialization by domain (Opus for backend, Gemini for UI, GLM for coherence) leverages model strengths - -### Filter Infrastructure -- Reusing existing filter infrastructure (ItemMigrationFilters, convertFilters) accelerates feature development (learned: 20260120-193522) -- Filter conversion layer between user-friendly format and API format provides clean separation of concerns (learned: 20260120-142840) -- Filter persistence in job metadata enables reproducibility (learned: 20260120-193522) - -### Validation & Error Handling -- Custom date validation with ISO 8601 support requires comprehensive edge case testing (learned: 20260120-142840) -- Backwards compatibility validation prevents breaking existing API consumers (learned: 20260120-193522) -- localStorage operations need QuotaExceededError and SecurityError handling (learned: 20260121-145015) -- Schema validation for JSON parsing prevents crashes from corrupted storage (learned: 20260121-145015) - -### State Persistence & Recovery -- localStorage persistence requires SSR safety with `typeof window` checks (learned: 20260121-145015) -- TTL-based expiration (24 hours) prevents stale data accumulation (learned: 20260121-145015) -- Auto-reconnect logic needs AbortController to prevent race conditions on component mount (learned: 20260121-145015) -- useMemo is better than useEffect for derived state like pagination (learned: 20260121-145015) +## Model Performance Notes -### React Hooks Best Practices -- Async useEffect must use AbortController + isMounted guard to prevent race conditions (learned: 20260122) -- Props used in useEffect body MUST be in dependency array - don't suppress with eslint-disable (learned: 20260122) -- Follow existing patterns in same file - if one useEffect uses AbortController, all async effects should (learned: 20260122) +- **Claude Opus**: Excellent orchestrator for multi-agent workflows, tie-breaker decisions, web standards research +- **OpenCode BigPickle**: Deep architecture analysis, pattern recognition +- **OpenCode GLM 4.7**: Code organization, architectural patterns +- **OpenCode Grok Code**: Fast search, test coverage analysis, learnings/standards scouting +- **OpenCode MiniMax M2.1**: Multi-language search, cross-file patterns +- **Gemini Flash**: Fast UI/frontend implementation +- **Codex GPT-5.2**: Code simplification while preserving functionality -### TypeScript Best Practices -- Extend interfaces with `Partial` instead of duplicating fields for optional inheritance (learned: 20260122) -- Import types from canonical location rather than redefining (learned: 20260122) +## Curated Guidelines -## Patterns That Failed +Based on the above patterns, future sessions should: -### eslint-disable for Dependency Arrays -- Suppressing `react-hooks/exhaustive-deps` with empty deps while using props leads to stale closures (learned: 20260122) -- The "run only on mount" pattern is usually wrong when props are involved - re-run is usually desired +1. **Use multi-agent verification for accessibility and standards questions** - 4 models provide diverse perspectives, Claude orchestrator resolves ties with authoritative sources +2. **Codify session guidelines upfront for complex features** - Guidelines prevent implementation drift across sequential workers +3. **Reuse existing infrastructure before building new** - Filter system, persistence patterns, validation layers +4. **Use useMemo for derived state, useRef + useEffect for DOM-only properties** - Tri-state logic, indeterminate checkboxes +5. **Never suppress react-hooks/exhaustive-deps** - Stale closures cause bugs, fix the deps array instead +6. **Async useEffect must use AbortController + isMounted guard** - Prevents race conditions +7. **localStorage needs defensive coding** - SSR checks, error handling, schema validation +8. **Don't add ARIA to native HTML elements** - Browsers handle accessibility automatically +9. **Research web standards (W3C/MDN) for tie-breaker decisions** - Authoritative sources resolve AI disagreements +10. **Follow existing patterns in the same file** - Consistency accelerates development and review -## Model Performance Notes +--- -- **Opus**: Excellent for backend/architecture work and resolving complex review findings -- **Gemini Flash**: Fast and capable for UI/frontend implementation -- **GLM**: Good for coherence verification between layers -- **BigPickle/Grok**: Effective parallel reviewers - catch complementary issues - -## Key Files & Patterns - -### Filter System -| File | Purpose | -|------|---------| -| `lib/migration/items/filter-converter.ts` | Converts user-friendly filters to Podio API format | -| `lib/migration/items/filter-validator.ts` | Validates filter inputs before API calls | -| `lib/migration/items/types.ts` | `ItemMigrationFilters` interface - reusable across features | - -### Cleanup Feature -| File | Purpose | -|------|---------| -| `lib/migration/cleanup/types.ts` | Cleanup request/response types | -| `lib/migration/cleanup/service.ts` | Duplicate detection with filter support | -| `lib/migration/cleanup/executor.ts` | Orchestrates cleanup workflow | -| `app/hooks/useCleanup.ts` | Cleanup state management with localStorage persistence | -| `app/components/migration/CleanupPanel.tsx` | Cleanup UI with job history panel | - -## Review Findings Categories - -Common issues caught by reviewers: -1. **Missing validation** - Date formats, empty filters, edge cases -2. **State handling bugs** - UI state not properly reset -3. **Backwards compatibility** - API changes breaking existing clients -4. **Performance** - Early returns for common cases (no filters) -5. **Reproducibility** - Storing config in job metadata - -## Sessions Curated - -| Session | Task | Outcome | -|---------|------|---------| -| 20260120-142840 | Add creation date filtering to item migrations | Success | -| 20260120-193522 | Add source filters to duplicate cleanup | Success | -| 20260121-145015 | Persist cleanup job state across page refresh | Success | -| 20260122 | Fix race conditions and type duplication (code review) | Success | +*Curated from 6 sessions spanning 2026-01-20 to 2026-01-22* From d6db2721723092af9a1b6b69e3068842cc4c2d2c Mon Sep 17 00:00:00 2001 From: rdfitted Date: Thu, 22 Jan 2026 16:06:36 -1000 Subject: [PATCH 3/5] docs: refine ARIA guideline to be more specific Update overly broad ARIA guideline in project-dna.md from "Don't add ARIA to native HTML elements" to more specific guidance: "Don't override native semantic state with ARIA". The refined guideline clarifies: - Valid ARIA usage: aria-label, aria-describedby for supplementary info - Invalid ARIA usage: aria-checked, aria-selected, aria-pressed on native elements - Rationale: Native checkboxes/radios/buttons expose state automatically Verified by 4 OpenCode agents (BigPickle, GLM 4.7, Grok Code, MiniMax M2.1) confirming the codebase uses valid ARIA attributes on native elements. Simplified by Codex GPT-5.2 for improved clarity. Curated 7 learnings and updated project-dna.md with new patterns. Resolves CodeRabbit PR comment on #50 Co-Authored-By: Codex GPT-5.2 --- .ai-docs/learnings.jsonl | 1 + .ai-docs/project-dna.md | 127 ++++++++++++++++++++++----------------- 2 files changed, 74 insertions(+), 54 deletions(-) diff --git a/.ai-docs/learnings.jsonl b/.ai-docs/learnings.jsonl index 1b5ac02..1d08eb6 100644 --- a/.ai-docs/learnings.jsonl +++ b/.ai-docs/learnings.jsonl @@ -4,3 +4,4 @@ {"date":"2026-01-22","session":"20260122-fix-comment","task":"Fix race conditions and type duplication from code review","outcome":"success","keywords":["AbortController","race-condition","stale-closure","useEffect","dependency-array","interface","extends","Partial","code-review","gemini-code-assist"],"insight":"Code review bots (gemini-code-assist) catch valid race condition issues. Three patterns validated: (1) async useEffect needs AbortController + isMounted guard, (2) props in effect body must be in deps array - dont suppress eslint, (3) extend interfaces with Partial instead of duplicating fields. Following existing patterns in same file accelerates fixes.","files_touched":["app/components/migration/CleanupPanel.tsx","app/hooks/useCleanup.ts","lib/migration/cleanup/types.ts"]} {"date":"2026-01-22","session":"20260122-144008-resolve-issue-49","task":"Resolve issue #49: Add master checkbox for bulk selection","outcome":"success","keywords":["checkbox","tri-state","useMemo","useRef","accessibility","react-hooks","indeterminate","bulk-selection","hive","multi-agent"],"insight":"Session guidelines codification upfront prevents implementation issues. Sequential workers (Opus->Gemini->GLM->Codex) execute cleanly when each follows shared guidelines. Tester phase catches edge cases missed by code reviewers (empty groups visibility). Key pattern: strong upfront guidelines + sequential layered approach + testing = clean implementation with zero reviewer issues.","files_touched":["app/components/migration/DuplicateGroupsPreview.tsx"]} {"date":"2026-01-22","session":"resolveprcomments-50","task":"Resolve PR comments","outcome":"success","keywords":["aria-checked","accessibility","native-checkbox","multi-agent-verification","PR-review","W3C","MDN","consensus-logic"],"insight":"Multi-agent verification (4 OpenCode models) with 2-2 tie resolution by Claude orchestrator works well for nuanced accessibility questions. Key finding: aria-checked should NOT be added to native HTML checkboxes - only to ARIA role='checkbox' elements. Native checkboxes expose indeterminate state automatically to assistive tech. Adding aria-checked to native inputs creates inconsistencies. Web standards research (W3C/MDN) essential for tie-breaker decisions.","files_touched":[]} +{"date":"2026-01-22","session":"resolveprcomments-50","task":"Resolve PR comments","outcome":"success","keywords":["ARIA","accessibility","native-elements","project-dna","documentation"],"insight":"Multi-agent verification (4 OpenCode models) confirmed CodeRabbit comment was VALID: the ARIA guideline was too broad. Updated from 'Don't add ARIA to native HTML elements' to more specific guidance about not overriding native semantic state. Valid ARIA usage (aria-label, aria-describedby) is allowed for supplementary info, but state attributes (aria-checked, aria-selected) should not duplicate native semantics. Codex GPT-5.2 further simplified the wording for clarity.","files_touched":[".ai-docs/project-dna.md"]} diff --git a/.ai-docs/project-dna.md b/.ai-docs/project-dna.md index 54e4fe5..070b436 100644 --- a/.ai-docs/project-dna.md +++ b/.ai-docs/project-dna.md @@ -1,46 +1,53 @@ # Project DNA - Podio Migration Agent *Last curated: 2026-01-22* -*Based on: 6 session learnings* +*Based on: 7 session learnings* ## Core Patterns ### What Works **Multi-Agent Workflows** -- Sequential worker pattern (implementation→coherence→simplification) ensures clean layer separation - Used in 3 sessions -- Parallel reviewers catch complementary issues (8 issues caught in one session) -- Session guidelines codification upfront prevents implementation issues - Zero reviewer issues when done right -- Multi-agent verification (4 models) with tie-breaker consensus works for nuanced decisions +- Sequential worker pattern (implementation→coherence→simplification) ensures clean layer separation - Used in 4 sessions +- Parallel reviewers catch complementary issues (8 issues caught in issue-37 session) +- Session guidelines codification upfront prevents implementation issues - Zero reviewer issues when done right (issue-49) +- Multi-agent verification (4 OpenCode models) with tie-breaker consensus works for nuanced decisions (PR-50) +- Sequential layered workers (Opus→Gemini→GLM→Codex) execute cleanly with shared guidelines -**Filter Infrastructure** +**Filter Infrastructure** (2 sessions: issue-34, issue-37) - Reusing existing filter infrastructure (ItemMigrationFilters, convertFilters) accelerates feature development - Filter conversion layer (user-friendly → API format) provides clean separation of concerns - Filter persistence in job metadata enables reproducibility +- Custom date validation with ISO 8601 support requires comprehensive edge case testing -**React Hooks Best Practices** +**React Hooks Best Practices** (3 sessions: issue-45, fix-comment, issue-49) - `useMemo` for derived state (tri-state logic, pagination) - better than `useEffect` - `useRef` + `useEffect` for DOM-only properties (`indeterminate`) - Async `useEffect` needs `AbortController` + `isMounted` guard to prevent race conditions - Props in effect body MUST be in dependency array - never suppress eslint - Follow existing patterns in same file for consistency -**State Persistence** +**State Persistence** (session: issue-45) - localStorage requires SSR safety (`typeof window` check), QuotaExceededError/SecurityError handling - Schema validation for JSON parsing prevents crashes from corrupted storage - TTL-based expiration prevents stale data accumulation - Auto-reconnect logic needs AbortController to prevent mount race conditions -**Accessibility Standards** +**Accessibility Standards** (2 sessions: PR-50) - `aria-checked` should NOT be added to native HTML checkboxes - only for ARIA `role="checkbox"` - Native checkboxes expose `indeterminate` state automatically to assistive tech +- Don't override native semantic state with ARIA attributes (aria-checked, aria-selected) - they duplicate native semantics +- Valid ARIA usage (aria-label, aria-describedby) is allowed for supplementary info - Web standards research (W3C/MDN) essential for accessibility decisions +**Code Review** +- Code review bots (gemini-code-assist, CodeRabbit) catch valid race condition and accessibility issues +- Multi-agent verification validates or refutes bot comments with research + **Validation & Error Handling** -- Custom date validation with ISO 8601 support requires comprehensive edge case testing - Schema validation with Zod accelerates API development -**TypeScript Patterns** +**TypeScript Patterns** (session: fix-comment) - Extend interfaces with `Partial` instead of duplicating fields ### What Doesn't Work @@ -52,23 +59,28 @@ **Field Duplication** - Duplicating interface fields instead of extending with `Partial` creates maintenance burden +**Overly Broad ARIA Guidelines** +- Blanket rule "Don't add ARIA to native HTML elements" is too broad - valid uses (aria-label, aria-describedby) exist +- Better: "Don't override native semantic state with ARIA" (more specific and actionable) + ## Hot Spots (Frequently Modified Files) -| File | Touch Count | Common Reason | -|------|-------------|---------------| -| `app/components/migration/CleanupPanel.tsx` | 3 | Cleanup feature evolution, race condition fixes, persistence | -| `lib/migration/cleanup/types.ts` | 3 | Filter integration, persistence schema, type fixes | -| `lib/migration/cleanup/service.ts` | 3 | Filter support, validation, executor integration | -| `app/hooks/useCleanup.ts` | 2 | Persistence, race condition fixes | -| `lib/migration/items/filter-converter.ts` | 1 | Initial implementation | -| `lib/migration/items/filter-validator.ts` | 1 | Initial implementation | -| `lib/ai/schemas/migration.ts` | 1 | Filter schema additions | -| `lib/ai/tools.ts` | 1 | Filter parameter additions | -| `lib/migration/items/service.ts` | 1 | Filter integration | -| `lib/migration/items/types.ts` | 1 | Filter types | -| `lib/migration/cleanup/executor.ts` | 1 | Filter persistence | -| `app/api/migration/cleanup/route.ts` | 1 | Filter API integration | -| `app/components/migration/DuplicateGroupsPreview.tsx` | 1 | Master checkbox tri-state implementation | +| File | Touch Count | Sessions | Common Reason | +|------|-------------|----------|---------------| +| `app/components/migration/CleanupPanel.tsx` | 3 | issue-37, issue-45, fix-comment | Cleanup feature evolution, race condition fixes, persistence | +| `lib/migration/cleanup/types.ts` | 3 | issue-37, issue-45, fix-comment | Filter integration, persistence schema, type fixes | +| `lib/migration/cleanup/service.ts` | 3 | issue-37, issue-45, fix-comment | Filter support, validation, executor integration | +| `app/hooks/useCleanup.ts` | 2 | issue-45, fix-comment | Persistence, race condition fixes | +| `lib/migration/items/filter-converter.ts` | 1 | issue-34 | Initial implementation | +| `lib/migration/items/filter-validator.ts` | 1 | issue-34 | Initial implementation | +| `lib/ai/schemas/migration.ts` | 1 | issue-34 | Filter schema additions | +| `lib/ai/tools.ts` | 1 | issue-34 | Filter parameter additions | +| `lib/migration/items/service.ts` | 1 | issue-34 | Filter integration | +| `lib/migration/items/types.ts` | 1 | issue-34 | Filter types | +| `lib/migration/cleanup/executor.ts` | 1 | issue-37 | Filter persistence | +| `app/api/migration/cleanup/route.ts` | 1 | issue-37 | Filter API integration | +| `app/components/migration/DuplicateGroupsPreview.tsx` | 1 | issue-49 | Master checkbox tri-state implementation | +| `.ai-docs/project-dna.md` | 1 | PR-50 | ARIA guideline refinement | *Files touched 3+ times indicate high complexity or rapid feature evolution (cleanup system).* @@ -76,59 +88,66 @@ | Cluster | Keywords | Sessions | |---------|----------|----------| -| **Cleanup System** | cleanup, duplicate, job-history, persistence, localStorage, TTL | 3 | -| **Filter Infrastructure** | filter, validation, date, schema, zod, api | 2 | -| **React Hooks** | useEffect, useMemo, useRef, AbortController, race-condition, dependency-array, stale-closure | 2 | -| **Accessibility** | accessibility, aria-checked, native-checkbox, indeterminate, tri-state | 2 | -| **Multi-Agent Orchestration** | multi-agent, hive, sequential-workers, parallel-reviewers, consensus-logic | 2 | +| **Cleanup System** | cleanup, duplicate, job-history, persistence, localStorage, TTL, pagination | 3 | +| **Filter Infrastructure** | filter, validation, date, schema, zod, api, podio | 3 | +| **React Hooks** | useEffect, useMemo, useRef, AbortController, race-condition, dependency-array, stale-closure | 3 | +| **Accessibility** | accessibility, aria-checked, native-checkbox, indeterminate, tri-state, ARIA, native-elements, W3C, MDN | 3 | +| **Multi-Agent Orchestration** | multi-agent, hive, sequential-workers, parallel-reviewers, consensus-logic, multi-agent-verification | 4 | | **Type Safety** | interface, extends, Partial, TypeScript | 1 | -| **Code Review** | code-review, gemini-code-assist, PR-review, W3C, MDN | 1 | -| **Podio API** | podio, backend, migration | 2 | -| **UI/Frontend** | ui, frontend, component, bulk-selection, checkbox | 2 | +| **Code Review** | code-review, gemini-code-assist, PR-review, CodeRabbit | 2 | +| **Podio API** | podio, backend, migration | 3 | +| **UI/Frontend** | ui, frontend, component, bulk-selection, checkbox | 3 | +| **Documentation** | project-dna, documentation | 1 | ## Session Insights (Deduplicated) -1. **Multi-agent hive with sequential workers and parallel reviewers works well for complex features** - Implementation workers focus on their layer, reviewers catch edge cases across all layers -2. **Filter conversion layer between user-friendly format and API format provides clean separation of concerns** - User sees ISO dates, API gets Podio filter keys -3. **Reusing existing infrastructure accelerates development** - ItemMigrationFilters reused for cleanup feature, MigrationContext pattern reused for cleanup state -4. **Custom validation requires comprehensive edge case testing** - ISO 8601 date formats, empty filters, malformed JSON -5. **localStorage persistence requires defensive coding** - SSR checks, error handling for quota/security, schema validation -6. **useMemo is better than useEffect for derived state** - Pagination, tri-state logic - no async, just compute -7. **Async useEffect needs AbortController + isMounted guard** - Prevents race conditions on mount/unmount -8. **Props in effect body must be in deps array** - Stale closures are the enemy, never suppress eslint -9. **Extend interfaces with Partial instead of duplicating fields** - Single source of truth -10. **Session guidelines codification upfront prevents implementation issues** - Strong guidelines + sequential workers + testing = zero reviewer issues -11. **Multi-agent verification with tie-breaker consensus works for nuanced questions** - 4 models vote, Claude orchestrator resolves 2-2 ties with research -12. **aria-checked is wrong for native checkboxes** - Native elements expose state automatically, adding ARIA creates inconsistencies -13. **Web standards research essential for accessibility decisions** - W3C/MDN authoritative sources resolve conflicting AI opinions -14. **Code review bots catch valid issues** - gemini-code-assist found race conditions and type duplication -15. **Following existing patterns in same file accelerates fixes** - If one effect uses AbortController, all should +1. **Multi-agent hive with sequential workers and parallel reviewers works well for complex features** - Implementation workers focus on their layer, reviewers catch edge cases across all layers (issue-34, issue-37) +2. **Filter conversion layer between user-friendly format and API format provides clean separation of concerns** - User sees ISO dates, API gets Podio filter keys (issue-34) +3. **Reusing existing infrastructure accelerates development** - ItemMigrationFilters reused for cleanup feature, MigrationContext pattern reused for cleanup state (issue-37, issue-45) +4. **Custom validation requires comprehensive edge case testing** - ISO 8601 date formats, empty filters, malformed JSON (issue-34, issue-37) +5. **localStorage persistence requires defensive coding** - SSR checks, error handling for quota/security, schema validation (issue-45) +6. **useMemo is better than useEffect for derived state** - Pagination, tri-state logic - no async, just compute (issue-45, issue-49) +7. **Async useEffect needs AbortController + isMounted guard** - Prevents race conditions on mount/unmount (issue-45, fix-comment) +8. **Props in effect body must be in deps array** - Stale closures are the enemy, never suppress eslint (fix-comment) +9. **Extend interfaces with Partial instead of duplicating fields** - Single source of truth (fix-comment) +10. **Session guidelines codification upfront prevents implementation issues** - Strong guidelines + sequential workers + testing = zero reviewer issues (issue-49) +11. **Multi-agent verification with tie-breaker consensus works for nuanced questions** - 4 models vote, Claude orchestrator resolves 2-2 ties with research (PR-50) +12. **aria-checked is wrong for native checkboxes** - Native elements expose state automatically, adding ARIA creates inconsistencies (PR-50) +13. **Web standards research essential for accessibility decisions** - W3C/MDN authoritative sources resolve conflicting AI opinions (PR-50) +14. **Code review bots catch valid issues** - gemini-code-assist and CodeRabbit found race conditions, type duplication, and overly broad guidelines (fix-comment, PR-50) +15. **Following existing patterns in same file accelerates fixes** - If one effect uses AbortController, all should (fix-comment) +16. **Sequential layered workers execute cleanly with shared guidelines** - Opus→Gemini→GLM→Codex pattern with upfront session guidelines (issue-49) +17. **Tester phase catches edge cases missed by code reviewers** - Empty groups visibility issue found during testing (issue-49) +18. **ARIA guidelines need specificity over breadth** - "Don't override native semantic state" is better than "Don't add ARIA to native elements" (PR-50) ## Model Performance Notes - **Claude Opus**: Excellent orchestrator for multi-agent workflows, tie-breaker decisions, web standards research -- **OpenCode BigPickle**: Deep architecture analysis, pattern recognition +- **OpenCode BigPickle**: Deep architecture analysis, pattern recognition, accessibility research - **OpenCode GLM 4.7**: Code organization, architectural patterns - **OpenCode Grok Code**: Fast search, test coverage analysis, learnings/standards scouting - **OpenCode MiniMax M2.1**: Multi-language search, cross-file patterns - **Gemini Flash**: Fast UI/frontend implementation -- **Codex GPT-5.2**: Code simplification while preserving functionality +- **Codex GPT-5.2**: Code simplification while preserving functionality, wording clarity ## Curated Guidelines Based on the above patterns, future sessions should: -1. **Use multi-agent verification for accessibility and standards questions** - 4 models provide diverse perspectives, Claude orchestrator resolves ties with authoritative sources +1. **Use multi-agent verification for accessibility and standards questions** - 4 models provide diverse perspectives, Claude orchestrator resolves ties with authoritative sources (W3C/MDN) 2. **Codify session guidelines upfront for complex features** - Guidelines prevent implementation drift across sequential workers 3. **Reuse existing infrastructure before building new** - Filter system, persistence patterns, validation layers 4. **Use useMemo for derived state, useRef + useEffect for DOM-only properties** - Tri-state logic, indeterminate checkboxes 5. **Never suppress react-hooks/exhaustive-deps** - Stale closures cause bugs, fix the deps array instead 6. **Async useEffect must use AbortController + isMounted guard** - Prevents race conditions 7. **localStorage needs defensive coding** - SSR checks, error handling, schema validation -8. **Don't add ARIA to native HTML elements** - Browsers handle accessibility automatically +8. **Don't override native semantic state with ARIA** - Native checkboxes/radios/buttons expose state automatically; only use ARIA for custom widgets or supplementary info (aria-label, aria-describedby) 9. **Research web standards (W3C/MDN) for tie-breaker decisions** - Authoritative sources resolve AI disagreements 10. **Follow existing patterns in the same file** - Consistency accelerates development and review +11. **Include tester phase in complex features** - Catches edge cases missed by code reviewers +12. **Sequential layered workers (Opus→Gemini→GLM→Codex) work well with shared guidelines** - Each layer focuses on its strength +13. **Trust but verify code review bots** - gemini-code-assist and CodeRabbit find valid issues, but verify with multi-agent research when conflicting --- -*Curated from 6 sessions spanning 2026-01-20 to 2026-01-22* +*Curated from 7 sessions spanning 2026-01-20 to 2026-01-22* From 0fa3d646b9e0eff9c1327c075f31c9dfc3adb270 Mon Sep 17 00:00:00 2001 From: rdfitted Date: Thu, 22 Jan 2026 16:38:05 -1000 Subject: [PATCH 4/5] docs: fix MD036 violations in project-dna.md MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Convert bold/italic emphasis used as headings to proper markdown heading syntax (####). Changes: - Convert **Section Name** to #### Section Name for all subsection headers - Update "What Works" section headers (11 fixes) - Update "What Doesn't Work" section headers (3 fixes) - Maintains proper heading hierarchy under ### parent sections - Aligns with project markdown standards (README.md, bug-patterns.md) Code simplification (Codex GPT-5.2): - Add spacing around arrows (→) for readability - Wrap technical terms in backticks for consistency - Improve grammatical accuracy Learning captured: - Multi-agent verification (4 OpenCode models) confirmed MD036 violations - Proper heading syntax provides semantic structure and consistency - Updated project-dna.md with new learning and curated insights Co-Authored-By: Codex GPT-5.2 --- .ai-docs/project-dna.md | 77 +++++++++++++++++++++-------------------- 1 file changed, 40 insertions(+), 37 deletions(-) diff --git a/.ai-docs/project-dna.md b/.ai-docs/project-dna.md index 070b436..a93231b 100644 --- a/.ai-docs/project-dna.md +++ b/.ai-docs/project-dna.md @@ -7,60 +7,61 @@ ### What Works -**Multi-Agent Workflows** -- Sequential worker pattern (implementation→coherence→simplification) ensures clean layer separation - Used in 4 sessions +#### Multi-Agent Workflows +- Sequential worker pattern (implementation → coherence → simplification) ensures clean layer separation - Used in 4 sessions - Parallel reviewers catch complementary issues (8 issues caught in issue-37 session) - Session guidelines codification upfront prevents implementation issues - Zero reviewer issues when done right (issue-49) - Multi-agent verification (4 OpenCode models) with tie-breaker consensus works for nuanced decisions (PR-50) -- Sequential layered workers (Opus→Gemini→GLM→Codex) execute cleanly with shared guidelines +- Sequential layered workers (Opus → Gemini → GLM → Codex) execute cleanly with shared guidelines -**Filter Infrastructure** (2 sessions: issue-34, issue-37) +#### Filter Infrastructure (issue-34, issue-37) - Reusing existing filter infrastructure (ItemMigrationFilters, convertFilters) accelerates feature development - Filter conversion layer (user-friendly → API format) provides clean separation of concerns - Filter persistence in job metadata enables reproducibility - Custom date validation with ISO 8601 support requires comprehensive edge case testing -**React Hooks Best Practices** (3 sessions: issue-45, fix-comment, issue-49) +#### React Hooks Best Practices (issue-45, fix-comment, issue-49) - `useMemo` for derived state (tri-state logic, pagination) - better than `useEffect` - `useRef` + `useEffect` for DOM-only properties (`indeterminate`) - Async `useEffect` needs `AbortController` + `isMounted` guard to prevent race conditions -- Props in effect body MUST be in dependency array - never suppress eslint +- Include all props used in an effect in the dependency array - never suppress ESLint - Follow existing patterns in same file for consistency -**State Persistence** (session: issue-45) -- localStorage requires SSR safety (`typeof window` check), QuotaExceededError/SecurityError handling +#### State Persistence (issue-45) +- `localStorage` requires SSR safety (`typeof window` check) and `QuotaExceededError`/`SecurityError` handling - Schema validation for JSON parsing prevents crashes from corrupted storage - TTL-based expiration prevents stale data accumulation -- Auto-reconnect logic needs AbortController to prevent mount race conditions +- Auto-reconnect logic needs `AbortController` to prevent mount race conditions -**Accessibility Standards** (2 sessions: PR-50) +#### Accessibility Standards (PR-50) - `aria-checked` should NOT be added to native HTML checkboxes - only for ARIA `role="checkbox"` - Native checkboxes expose `indeterminate` state automatically to assistive tech -- Don't override native semantic state with ARIA attributes (aria-checked, aria-selected) - they duplicate native semantics -- Valid ARIA usage (aria-label, aria-describedby) is allowed for supplementary info +- Don't override native semantic state with ARIA attributes (`aria-checked`, `aria-selected`) - they duplicate native semantics +- Valid ARIA usage (`aria-label`, `aria-describedby`) is allowed for supplementary info - Web standards research (W3C/MDN) essential for accessibility decisions -**Code Review** -- Code review bots (gemini-code-assist, CodeRabbit) catch valid race condition and accessibility issues +#### Code Review +- Code review bots (gemini-code-assist, CodeRabbit) catch valid race condition, accessibility, and documentation issues - Multi-agent verification validates or refutes bot comments with research +- Markdown documentation should use proper heading syntax (`####`) for semantic structure, not bold emphasis - aligns with project standards (README.md, bug-patterns.md) -**Validation & Error Handling** +#### Validation & Error Handling - Schema validation with Zod accelerates API development -**TypeScript Patterns** (session: fix-comment) +#### TypeScript Patterns (fix-comment) - Extend interfaces with `Partial` instead of duplicating fields ### What Doesn't Work -**eslint-disable for Dependency Arrays** +#### `eslint-disable` for Dependency Arrays - Suppressing `react-hooks/exhaustive-deps` with empty deps while using props leads to stale closures - The "run only on mount" pattern is wrong when props are involved -**Field Duplication** +#### Field Duplication - Duplicating interface fields instead of extending with `Partial` creates maintenance burden -**Overly Broad ARIA Guidelines** -- Blanket rule "Don't add ARIA to native HTML elements" is too broad - valid uses (aria-label, aria-describedby) exist +#### Overly Broad ARIA Guidelines +- Blanket rule "Don't add ARIA to native HTML elements" is too broad - valid uses (`aria-label`, `aria-describedby`) exist - Better: "Don't override native semantic state with ARIA" (more specific and actionable) ## Hot Spots (Frequently Modified Files) @@ -80,9 +81,9 @@ | `lib/migration/cleanup/executor.ts` | 1 | issue-37 | Filter persistence | | `app/api/migration/cleanup/route.ts` | 1 | issue-37 | Filter API integration | | `app/components/migration/DuplicateGroupsPreview.tsx` | 1 | issue-49 | Master checkbox tri-state implementation | -| `.ai-docs/project-dna.md` | 1 | PR-50 | ARIA guideline refinement | +| `.ai-docs/project-dna.md` | 2 | PR-50 (2 fixes) | ARIA guideline refinement, MD036 heading syntax fixes | -*Files touched 3+ times indicate high complexity or rapid feature evolution (cleanup system).* +*Files touched 3+ times indicate high complexity or rapid feature evolution, especially in the cleanup system.* ## Keyword Clusters @@ -97,7 +98,7 @@ | **Code Review** | code-review, gemini-code-assist, PR-review, CodeRabbit | 2 | | **Podio API** | podio, backend, migration | 3 | | **UI/Frontend** | ui, frontend, component, bulk-selection, checkbox | 3 | -| **Documentation** | project-dna, documentation | 1 | +| **Documentation** | project-dna, documentation, markdownlint, markdown-headings | 2 | ## Session Insights (Deduplicated) @@ -105,20 +106,21 @@ 2. **Filter conversion layer between user-friendly format and API format provides clean separation of concerns** - User sees ISO dates, API gets Podio filter keys (issue-34) 3. **Reusing existing infrastructure accelerates development** - ItemMigrationFilters reused for cleanup feature, MigrationContext pattern reused for cleanup state (issue-37, issue-45) 4. **Custom validation requires comprehensive edge case testing** - ISO 8601 date formats, empty filters, malformed JSON (issue-34, issue-37) -5. **localStorage persistence requires defensive coding** - SSR checks, error handling for quota/security, schema validation (issue-45) -6. **useMemo is better than useEffect for derived state** - Pagination, tri-state logic - no async, just compute (issue-45, issue-49) -7. **Async useEffect needs AbortController + isMounted guard** - Prevents race conditions on mount/unmount (issue-45, fix-comment) -8. **Props in effect body must be in deps array** - Stale closures are the enemy, never suppress eslint (fix-comment) -9. **Extend interfaces with Partial instead of duplicating fields** - Single source of truth (fix-comment) +5. **`localStorage` persistence requires defensive coding** - SSR checks, error handling for quota/security, schema validation (issue-45) +6. **`useMemo` is better than `useEffect` for derived state** - Pagination, tri-state logic - no async, just compute (issue-45, issue-49) +7. **Async `useEffect` needs `AbortController` + `isMounted` guard** - Prevents race conditions on mount/unmount (issue-45, fix-comment) +8. **Props in effect body must be in deps array** - Stale closures are the enemy, never suppress ESLint (fix-comment) +9. **Extend interfaces with `Partial` instead of duplicating fields** - Single source of truth (fix-comment) 10. **Session guidelines codification upfront prevents implementation issues** - Strong guidelines + sequential workers + testing = zero reviewer issues (issue-49) 11. **Multi-agent verification with tie-breaker consensus works for nuanced questions** - 4 models vote, Claude orchestrator resolves 2-2 ties with research (PR-50) -12. **aria-checked is wrong for native checkboxes** - Native elements expose state automatically, adding ARIA creates inconsistencies (PR-50) +12. **`aria-checked` is wrong for native checkboxes** - Native elements expose state automatically, adding ARIA creates inconsistencies (PR-50) 13. **Web standards research essential for accessibility decisions** - W3C/MDN authoritative sources resolve conflicting AI opinions (PR-50) 14. **Code review bots catch valid issues** - gemini-code-assist and CodeRabbit found race conditions, type duplication, and overly broad guidelines (fix-comment, PR-50) -15. **Following existing patterns in same file accelerates fixes** - If one effect uses AbortController, all should (fix-comment) -16. **Sequential layered workers execute cleanly with shared guidelines** - Opus→Gemini→GLM→Codex pattern with upfront session guidelines (issue-49) +15. **Following existing patterns in the same file accelerates fixes** - If one effect uses `AbortController`, all should (fix-comment) +16. **Sequential layered workers execute cleanly with shared guidelines** - Opus → Gemini → GLM → Codex pattern with upfront session guidelines (issue-49) 17. **Tester phase catches edge cases missed by code reviewers** - Empty groups visibility issue found during testing (issue-49) 18. **ARIA guidelines need specificity over breadth** - "Don't override native semantic state" is better than "Don't add ARIA to native elements" (PR-50) +19. **Markdown documentation should use proper heading syntax** - Converting `**Section Name**` to `#### Section Name` provides semantic structure and aligns with project standards (PR-50) ## Model Performance Notes @@ -137,16 +139,17 @@ Based on the above patterns, future sessions should: 1. **Use multi-agent verification for accessibility and standards questions** - 4 models provide diverse perspectives, Claude orchestrator resolves ties with authoritative sources (W3C/MDN) 2. **Codify session guidelines upfront for complex features** - Guidelines prevent implementation drift across sequential workers 3. **Reuse existing infrastructure before building new** - Filter system, persistence patterns, validation layers -4. **Use useMemo for derived state, useRef + useEffect for DOM-only properties** - Tri-state logic, indeterminate checkboxes -5. **Never suppress react-hooks/exhaustive-deps** - Stale closures cause bugs, fix the deps array instead -6. **Async useEffect must use AbortController + isMounted guard** - Prevents race conditions -7. **localStorage needs defensive coding** - SSR checks, error handling, schema validation -8. **Don't override native semantic state with ARIA** - Native checkboxes/radios/buttons expose state automatically; only use ARIA for custom widgets or supplementary info (aria-label, aria-describedby) +4. **Use `useMemo` for derived state, `useRef` + `useEffect` for DOM-only properties** - Tri-state logic, indeterminate checkboxes +5. **Never suppress `react-hooks/exhaustive-deps`** - Stale closures cause bugs, fix the deps array instead +6. **Async `useEffect` must use `AbortController` + `isMounted` guard** - Prevents race conditions +7. **`localStorage` needs defensive coding** - SSR checks, error handling, schema validation +8. **Don't override native semantic state with ARIA** - Native checkboxes/radios/buttons expose state automatically; only use ARIA for custom widgets or supplementary info (`aria-label`, `aria-describedby`) 9. **Research web standards (W3C/MDN) for tie-breaker decisions** - Authoritative sources resolve AI disagreements 10. **Follow existing patterns in the same file** - Consistency accelerates development and review 11. **Include tester phase in complex features** - Catches edge cases missed by code reviewers -12. **Sequential layered workers (Opus→Gemini→GLM→Codex) work well with shared guidelines** - Each layer focuses on its strength +12. **Sequential layered workers (Opus → Gemini → GLM → Codex) work well with shared guidelines** - Each layer focuses on its strength 13. **Trust but verify code review bots** - gemini-code-assist and CodeRabbit find valid issues, but verify with multi-agent research when conflicting +14. **Use proper markdown heading syntax in documentation** - Headings should use `#`/`##`/`###`/`####` syntax rather than bold emphasis for semantic structure and consistency --- From 072fadf5e221b55283f01bc78935522421b4169d Mon Sep 17 00:00:00 2001 From: rdfitted Date: Fri, 23 Jan 2026 11:58:26 -1000 Subject: [PATCH 5/5] fix: skip re-detection when executing with approved groups When clicking "Proceed" after a dry run, the executor was unnecessarily re-scanning the entire app even though approved groups were already provided. This caused: - Status to regress to "detecting" - Risk of getting stuck if scan was interrupted - Wasted time re-scanning large datasets Now when approvedGroups are provided, detection is skipped entirely and execution proceeds directly with the pre-approved groups. Co-Authored-By: Claude Opus 4.5 --- lib/migration/cleanup/executor.ts | 53 ++++++++++++++++++++----------- 1 file changed, 35 insertions(+), 18 deletions(-) diff --git a/lib/migration/cleanup/executor.ts b/lib/migration/cleanup/executor.ts index b0fa9da..e6db56d 100644 --- a/lib/migration/cleanup/executor.ts +++ b/lib/migration/cleanup/executor.ts @@ -109,30 +109,47 @@ export class CleanupExecutor extends EventEmitter { matchField: this.config.matchField, mode: this.config.mode, dryRun: this.config.dryRun, + hasApprovedGroups: !!(this.config.approvedGroups && this.config.approvedGroups.length > 0), }); - // Update job status to detecting - await migrationStateStore.updateJobStatus(this.jobId, 'detecting' as any); + // OPTIMIZATION: Skip detection if approved groups are already provided + // This happens when user clicks "Proceed" after reviewing dry run results + let limitedGroups: DuplicateGroup[]; - // Step 1 & 2: Stream items and detect duplicate groups efficiently - this.emit('detectStart'); - const duplicateGroups = await detectDuplicateGroups( - this.client, - this.config.appId, - this.config.matchField, - { + if (this.config.approvedGroups && this.config.approvedGroups.length > 0) { + // Use pre-approved groups - skip expensive re-detection + logger.info('Using pre-approved groups, skipping detection', { jobId: this.jobId, - onPauseCheck: () => this.pauseRequested, - filters: this.config.filters, - } - ); + approvedGroupsCount: this.config.approvedGroups.length, + }); + + await migrationStateStore.updateJobStatus(this.jobId, 'in_progress' as any); + limitedGroups = this.config.approvedGroups; + this.emit('detectComplete', limitedGroups); + } else { + // No approved groups - run full detection + await migrationStateStore.updateJobStatus(this.jobId, 'detecting' as any); + + // Step 1 & 2: Stream items and detect duplicate groups efficiently + this.emit('detectStart'); + const duplicateGroups = await detectDuplicateGroups( + this.client, + this.config.appId, + this.config.matchField, + { + jobId: this.jobId, + onPauseCheck: () => this.pauseRequested, + filters: this.config.filters, + } + ); - // Apply max groups limit if specified - const limitedGroups = this.config.maxGroups - ? duplicateGroups.slice(0, this.config.maxGroups) - : duplicateGroups; + // Apply max groups limit if specified + limitedGroups = this.config.maxGroups + ? duplicateGroups.slice(0, this.config.maxGroups) + : duplicateGroups; - this.emit('detectComplete', limitedGroups); + this.emit('detectComplete', limitedGroups); + } // Step 3: Determine which items to delete let groupsToProcess: DuplicateGroup[];