Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -85,14 +85,14 @@ export class OrchestrationStateManager {

if (pendingDeps.length === 0) {
// Check if task has an assignee
const task = this.state.taskQueue.find((t) => t.id === taskId);
const task = this.getTask(taskId);
if (task?.assignee) {
plan.ready.push(taskId);
}
} else {
// Check for failed dependencies
// Check for failed dependencies (use getTask to search both taskQueue and completedTasks)
const failedDeps = pendingDeps.filter((depId) => {
const task = this.state.taskQueue.find((t) => t.id === depId);
const task = this.getTask(depId);
return task?.status === 'cancelled';
});

Expand Down
1 change: 0 additions & 1 deletion sonar-project.properties
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ sonar.exclusions=**/node_modules/**,**/dist/**,**/coverage/**,**/*.test.ts,**/*.

# Coverage
sonar.javascript.lcov.reportPaths=coverage/lcov.info
# Note: app/_layout.tsx is excluded because its routing logic is tested via Playwright E2E tests
sonar.coverage.exclusions=**/*.test.ts,**/*.test.tsx,**/*.spec.ts,**/*.spec.tsx,**/*.config.js,**/*.config.ts,**/__tests__/**,**/__mocks__/**

# TypeScript
Expand Down
13 changes: 10 additions & 3 deletions src/components/code/DiffViewer.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ interface DiffViewerProps {
showLineNumbers?: boolean;
}

function parseDiff(oldContent: string, newContent: string): DiffLine[] {
export function parseDiff(oldContent: string, newContent: string): DiffLine[] {
// Simple line-by-line diff (in production, use a proper diff library)
const oldLines = oldContent.split('\n');
const newLines = newContent.split('\n');
Expand Down Expand Up @@ -99,8 +99,15 @@ export function DiffViewer({
() => diff || (oldContent && newContent ? parseDiff(oldContent, newContent) : []),
[diff, oldContent, newContent]
);
const additions = useMemo(() => lines.filter((l) => l.type === 'add').length, [lines]);
const deletions = useMemo(() => lines.filter((l) => l.type === 'remove').length, [lines]);
const [additions, deletions] = useMemo(() => {
let adds = 0;
let dels = 0;
for (const line of lines) {
if (line.type === 'add') adds++;
else if (line.type === 'remove') dels++;
}
return [adds, dels];
}, [lines]);

const getLineStyle = (type: DiffLine['type']) => {
switch (type) {
Expand Down
2 changes: 1 addition & 1 deletion src/components/code/FileTree.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ export function FileTree({
);

useEffect(() => {
store.getState().setSelectedPath(selectedPath || '');
store.getState().setSelectedPath(selectedPath);
}, [selectedPath, store]);

// Sort nodes: folders first, then alphabetically
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,8 @@
import { act, fireEvent, render, screen } from '@testing-library/react';
import { DiffViewer } from '../DiffViewer';
import { beforeEach, describe, expect, it, vi } from 'vitest';
import * as DiffViewerModule from '../DiffViewer';

const { DiffViewer } = DiffViewerModule;

// Mock dependencies to isolate DiffViewer performance
vi.mock('@/components/icons', () => ({
Expand All @@ -16,8 +19,40 @@
}));

describe('DiffViewer Performance', () => {
beforeEach(() => {
vi.restoreAllMocks();
});

it('does not re-parse diff on re-render (memoization verification)', async () => {
const linesCount = 100;
const oldContent = Array.from({ length: linesCount }, (_, i) => `original line ${i}`).join(
'\n'
);
const newContent = Array.from({ length: linesCount }, (_, i) => `modified line ${i}`).join(
'\n'
);

// Spy on parseDiff to verify memoization
const parseDiffSpy = vi.spyOn(DiffViewerModule, 'parseDiff');

// Initial render
render(<DiffViewer oldContent={oldContent} newContent={newContent} filename="benchmark.ts" />);

const initialCallCount = parseDiffSpy.mock.calls.length;
expect(initialCallCount).toBeGreaterThan(0);

Check failure on line 42 in src/components/code/__tests__/diff-viewer.perf.test.tsx

View workflow job for this annotation

GitHub Actions / Run Tests

src/components/code/__tests__/diff-viewer.perf.test.tsx > DiffViewer Performance > does not re-parse diff on re-render (memoization verification)

AssertionError: expected 0 to be greater than 0 ❯ src/components/code/__tests__/diff-viewer.perf.test.tsx:42:30

Check failure on line 42 in src/components/code/__tests__/diff-viewer.perf.test.tsx

View workflow job for this annotation

GitHub Actions / Run Tests

src/components/code/__tests__/diff-viewer.perf.test.tsx > DiffViewer Performance > does not re-parse diff on re-render (memoization verification)

AssertionError: expected 0 to be greater than 0 ❯ src/components/code/__tests__/diff-viewer.perf.test.tsx:42:30

const button = screen.getByRole('button');

// Trigger re-render by collapsing (should NOT call parseDiff again)
await act(async () => {
fireEvent.click(button);
});

// Verify parseDiff was NOT called again — memoization is working
expect(parseDiffSpy.mock.calls.length).toBe(initialCallCount);
});

it('measures re-render time with large diffs', async () => {
// Generate large content (~5000 lines) to make parsing expensive
const linesCount = 5000;
const oldContent = Array.from({ length: linesCount }, (_, i) => `original line ${i}`).join(
'\n'
Expand All @@ -44,7 +79,7 @@
const updateTime = endUpdate - startUpdate;
console.log(`[Benchmark] Re-render time (collapse): ${updateTime.toFixed(2)}ms`);

// Sanity check
expect(updateTime).toBeGreaterThan(0);
// Re-render should be significantly faster than initial render
expect(updateTime).toBeLessThan(endRender - startRender);
}, 10000);
});
36 changes: 35 additions & 1 deletion src/components/feedback/Loading.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,40 @@ export function Skeleton({
);
}

/** Pre-composed skeleton for a text block with multiple lines */
export function SkeletonText({
lines = 3,
lastLineWidth = '60%',
}: Readonly<{ lines?: number; lastLineWidth?: string | number }>) {
return (
<div className="flex flex-col gap-2">
{Array.from({ length: lines }, (_, i) => (
<Skeleton
key={`skel-line-${i}`}
width={i === lines - 1 ? lastLineWidth : '100%'}
height={14}
/>
))}
</div>
);
}

/** Pre-composed skeleton for a card layout (avatar + text) */
export function SkeletonCard() {
return (
<div className="bg-surface p-4 rounded-organic-card animate-pulse">
<div className="flex items-center gap-3 mb-3">
<Skeleton circle height={40} />
<div className="flex-1 flex flex-col gap-2">
<Skeleton width="50%" height={14} />
<Skeleton width="30%" height={12} />
</div>
</div>
<SkeletonText lines={2} />
</div>
);
}

interface LoadingOverlayProps {
/** Whether the overlay is visible */
visible: boolean;
Expand All @@ -80,7 +114,7 @@ export function LoadingOverlay({ visible, message }: Readonly<LoadingOverlayProp
<div className="absolute inset-0 bg-charcoal/80 flex items-center justify-center z-50">
<div
className="bg-surface-elevated p-6 flex flex-col items-center"
style={{ borderRadius: '16px 12px 20px 8px' }}
style={{ borderRadius: '1rem 0.75rem 1.25rem 0.5rem' }}
Comment on lines 116 to +117

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

While this change correctly updates the border-radius to use rem units, it misses the opportunity to fully embrace the new Tailwind configuration. To improve maintainability and align with the goal of replacing inline styles, it would be better to use the rounded-organic-card class directly, which is now available in tailwind.config.ts.

Suggested change
className="bg-surface-elevated p-6 flex flex-col items-center"
style={{ borderRadius: '16px 12px 20px 8px' }}
style={{ borderRadius: '1rem 0.75rem 1.25rem 0.5rem' }}
className="bg-surface-elevated p-6 flex flex-col items-center rounded-organic-card"

>
<Spinner size="lg" />
{message && <span className="font-body text-white mt-4 text-center">{message}</span>}
Expand Down
133 changes: 133 additions & 0 deletions src/components/feedback/Pagination.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,133 @@
/**
* Pagination Component
*
* Provides page navigation for long lists and data tables.
* Uses organic styling for brand consistency.
*/

import { ChevronLeft, ChevronRight } from 'lucide-react';

interface PaginationProps {
/** Current page (1-indexed) */
currentPage: number;
/** Total number of pages */
totalPages: number;
/** Callback when page changes */
onPageChange: (page: number) => void;
/** Maximum number of page buttons to show */
maxVisible?: number;
}

export function Pagination({
currentPage,
totalPages,
onPageChange,
maxVisible = 5,
}: Readonly<PaginationProps>) {
if (totalPages <= 1) return null;

const getVisiblePages = (): number[] => {
if (totalPages <= maxVisible) {
return Array.from({ length: totalPages }, (_, i) => i + 1);
}

const half = Math.floor(maxVisible / 2);
let start = Math.max(1, currentPage - half);
const end = Math.min(totalPages, start + maxVisible - 1);

if (end - start + 1 < maxVisible) {
start = Math.max(1, end - maxVisible + 1);
}

return Array.from({ length: end - start + 1 }, (_, i) => start + i);
};

const pages = getVisiblePages();
const hasPrev = currentPage > 1;
const hasNext = currentPage < totalPages;

return (
<nav className="flex items-center justify-center gap-1" aria-label="Pagination">
<button
type="button"
onClick={() => onPageChange(currentPage - 1)}
disabled={!hasPrev}
className={`p-2 rounded-organic-button transition-colors ${
hasPrev
? 'text-neutral-300 hover:bg-surface-elevated hover:text-white'
: 'text-neutral-600 cursor-not-allowed'
}`}
aria-label="Previous page"
>
<ChevronLeft size={16} />
</button>

{pages[0] > 1 && (
<>
<button
type="button"
onClick={() => onPageChange(1)}
className="w-9 h-9 flex items-center justify-center rounded-organic-button font-body text-sm text-neutral-300 hover:bg-surface-elevated hover:text-white transition-colors"
aria-label="Page 1"
>
1
</button>
{pages[0] > 2 && (
<span className="w-9 h-9 flex items-center justify-center text-neutral-500 font-body text-sm">
...
</span>
)}
</>
)}

{pages.map((page) => (
<button
type="button"
key={page}
onClick={() => onPageChange(page)}
className={`w-9 h-9 flex items-center justify-center rounded-organic-button font-body text-sm transition-colors ${
page === currentPage
? 'bg-coral-500 text-white font-semibold'
: 'text-neutral-300 hover:bg-surface-elevated hover:text-white'
}`}
aria-label={`Page ${page}`}
aria-current={page === currentPage ? 'page' : undefined}
>
{page}
</button>
))}

{pages[pages.length - 1] < totalPages && (
<>
{pages[pages.length - 1] < totalPages - 1 && (
<span className="w-9 h-9 flex items-center justify-center text-neutral-500 font-body text-sm">
...
</span>
)}
<button
type="button"
onClick={() => onPageChange(totalPages)}
className="w-9 h-9 flex items-center justify-center rounded-organic-button font-body text-sm text-neutral-300 hover:bg-surface-elevated hover:text-white transition-colors"
aria-label={`Page ${totalPages}`}
>
{totalPages}
</button>
</>
)}

<button
type="button"
onClick={() => onPageChange(currentPage + 1)}
disabled={!hasNext}
className={`p-2 rounded-organic-button transition-colors ${
hasNext
? 'text-neutral-300 hover:bg-surface-elevated hover:text-white'
: 'text-neutral-600 cursor-not-allowed'
}`}
aria-label="Next page"
>
<ChevronRight size={16} />
</button>
</nav>
);
}
3 changes: 2 additions & 1 deletion src/components/feedback/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@
*/

export { ActionSheet, BottomSheet } from './BottomSheet';
export { LoadingOverlay, Skeleton, Spinner } from './Loading';
export { LoadingOverlay, Skeleton, SkeletonCard, SkeletonText, Spinner } from './Loading';
export { ConfirmDialog, Modal } from './Modal';
export { Pagination } from './Pagination';
export { ProgressBar, ProgressCircle, StepsProgress } from './Progress';
export { Toast } from './Toast';
6 changes: 3 additions & 3 deletions src/pages/NotFound.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ export function NotFoundPage() {
<div className="max-w-sm w-full">
<div className="flex flex-col items-center space-y-6">
{/* Icon */}
<div className="w-24 h-24 bg-surface flex items-center justify-center rounded-[28px_24px_32px_20px/20px_28px_24px_32px]">
<div className="w-24 h-24 bg-surface flex items-center justify-center rounded-organic-hero">
<Search size={48} className="text-neutral-500" />
</div>

Expand All @@ -32,15 +32,15 @@ export function NotFoundPage() {
<div className="w-full space-y-3">
<Link
to="/"
className="block w-full py-4 px-8 bg-coral-500 text-center text-white font-semibold font-body rounded-[14px_16px_12px_18px/12px_14px_16px_10px] hover:bg-coral-600 transition-colors"
className="block w-full py-4 px-8 bg-coral-500 text-center text-white font-semibold font-body rounded-organic-cta hover:bg-coral-600 transition-colors"
>
Go Home
</Link>

<button
type="button"
onClick={() => navigate(-1)}
className="block w-full py-4 px-8 bg-surface text-center text-white font-body rounded-[14px_16px_12px_18px/12px_14px_16px_10px] hover:bg-neutral-700 transition-colors"
className="block w-full py-4 px-8 bg-surface text-center text-white font-body rounded-organic-cta hover:bg-neutral-700 transition-colors"
>
Go Back
</button>
Expand Down
Loading
Loading