feat(subtask): enhance subtask view to intelligently surface the most relevant actions from the logs #331
Conversation
📝 WalkthroughWalkthroughAdds an action-scoring library, tests, i18n keys, and UI components to surface top‑N scored subtask actions; wires Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant TDM as TaskDetailModal
participant TSub as TaskSubtasks
participant AS as ActionScoring
participant SAL as SubtaskActionList
participant UI as Rendered UI
User->>TDM: Open task detail
TDM->>TSub: pass phaseLogs, onViewAllLogs
rect rgba(248,251,255,0.9)
Note over AS,TSub: Aggregate & score entries
TSub->>AS: getAllEntriesFromPhaseLogs(phaseLogs)
AS-->>TSub: TaskLogEntry[]
TSub->>AS: calculateSubtaskRelevanceScores(entries, subtaskIds)
AS-->>TSub: Map<subtaskId, RelevanceScore>
end
rect rgba(255,248,240,0.9)
Note over TSub,UI: Order & render subtasks
TSub->>TSub: sort subtasks (relevance toggle)
TSub-->>UI: render headers/badges
end
User->>UI: Expand a subtask
UI->>TSub: expand event
rect rgba(246,255,246,0.9)
Note over SAL,AS: Top‑N selection & tooltips
TSub->>AS: filterTopActions(subtaskActions, topN)
AS-->>SAL: ScoredAction[]
SAL->>AS: getScoreReason(scoredAction)
SAL-->>UI: render actions, files, score tooltip
end
User->>UI: Click "View all logs"
UI->>TSub: onViewAllLogs()
TSub->>TDM: trigger tab change to Logs
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro 📒 Files selected for processing (7)
🧰 Additional context used📓 Path-based instructions (3)apps/frontend/src/**/*.{ts,tsx}📄 CodeRabbit inference engine (CLAUDE.md)
Files:
apps/frontend/**/*.{ts,tsx}⚙️ CodeRabbit configuration file
Files:
apps/frontend/src/shared/i18n/locales/**/*.json📄 CodeRabbit inference engine (CLAUDE.md)
Files:
🧠 Learnings (3)📚 Learning: 2025-12-25T18:29:32.954ZApplied to files:
📚 Learning: 2025-12-25T18:29:32.954ZApplied to files:
📚 Learning: 2025-12-25T18:29:32.954ZApplied to files:
🧬 Code graph analysis (3)apps/frontend/src/renderer/components/task-detail/TaskDetailModal.tsx (1)
apps/frontend/src/renderer/lib/__tests__/actionScoring.test.ts (2)
apps/frontend/src/renderer/lib/actionScoring.ts (1)
🔇 Additional comments (25)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 30
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
apps/frontend/src/renderer/components/task-detail/TaskSubtasks.tsx (1)
125-131: Use translation keys for user-facing text.As per coding guidelines, all user-facing text in React/TypeScript frontend components must use translation keys with
useTranslation(). Multiple hardcoded strings in this component violate this requirement.Key violations:
- Lines 127-129: "No subtasks defined", "Implementation subtasks will appear here after planning"
- Lines 159, 166-167: Sort mode labels and tooltips
- Lines 237-244: Relevance score labels
Example fix for empty state
+import { useTranslation } from 'react-i18next'; + export function TaskSubtasks({ task, phaseLogs, onViewAllLogs }: TaskSubtasksProps) { + const { t } = useTranslation('taskDetail'); const progress = calculateProgress(task.subtasks); // ... // In JSX: - <p className="text-sm font-medium text-muted-foreground mb-1">No subtasks defined</p> - <p className="text-xs text-muted-foreground/70"> - Implementation subtasks will appear here after planning - </p> + <p className="text-sm font-medium text-muted-foreground mb-1">{t('subtasks.empty.title')}</p> + <p className="text-xs text-muted-foreground/70"> + {t('subtasks.empty.description')} + </p>
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
⛔ Files ignored due to path filters (1)
apps/frontend/package-lock.jsonis excluded by!**/package-lock.json,!**/package-lock.json
📒 Files selected for processing (29)
apps/frontend/.env.exampleapps/frontend/package.jsonapps/frontend/src/__tests__/integration/api-ipc-bridge.test.tsapps/frontend/src/__tests__/integration/openapi-spec.test.tsapps/frontend/src/main/api/.gitkeepapps/frontend/src/main/api/__tests__/websocket.spec.tsapps/frontend/src/main/api/event-bridge.tsapps/frontend/src/main/api/index.tsapps/frontend/src/main/api/middleware/.gitkeepapps/frontend/src/main/api/middleware/__tests__/auth.spec.tsapps/frontend/src/main/api/middleware/auth.tsapps/frontend/src/main/api/routes/.gitkeepapps/frontend/src/main/api/routes/__tests__/tasks.spec.tsapps/frontend/src/main/api/routes/index.tsapps/frontend/src/main/api/routes/monitoring.tsapps/frontend/src/main/api/routes/projects.tsapps/frontend/src/main/api/routes/tasks.tsapps/frontend/src/main/api/schemas/.gitkeepapps/frontend/src/main/api/schemas/index.tsapps/frontend/src/main/api/server.tsapps/frontend/src/main/api/startup.tsapps/frontend/src/main/api/types/index.tsapps/frontend/src/main/api/websocket.tsapps/frontend/src/main/index.tsapps/frontend/src/renderer/components/task-detail/SubtaskActionList.tsxapps/frontend/src/renderer/components/task-detail/TaskDetailModal.tsxapps/frontend/src/renderer/components/task-detail/TaskSubtasks.tsxapps/frontend/src/renderer/lib/__tests__/actionScoring.test.tsapps/frontend/src/renderer/lib/actionScoring.ts
🧰 Additional context used
📓 Path-based instructions (2)
apps/frontend/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/frontend/src/**/*.{ts,tsx}: Always use translation keys withuseTranslation()for all user-facing text in React/TypeScript frontend components - use formatnamespace:section.key(e.g.,navigation:items.githubPRs)
Never use hardcoded strings in JSX/TSX files for user-facing text - always reference translation keys fromapps/frontend/src/shared/i18n/locales/
Files:
apps/frontend/src/main/api/middleware/__tests__/auth.spec.tsapps/frontend/src/main/api/routes/tasks.tsapps/frontend/src/renderer/components/task-detail/TaskDetailModal.tsxapps/frontend/src/renderer/components/task-detail/SubtaskActionList.tsxapps/frontend/src/__tests__/integration/api-ipc-bridge.test.tsapps/frontend/src/main/api/server.tsapps/frontend/src/main/api/event-bridge.tsapps/frontend/src/main/api/types/index.tsapps/frontend/src/main/api/routes/projects.tsapps/frontend/src/main/api/routes/__tests__/tasks.spec.tsapps/frontend/src/__tests__/integration/openapi-spec.test.tsapps/frontend/src/main/api/routes/index.tsapps/frontend/src/main/api/middleware/auth.tsapps/frontend/src/main/api/websocket.tsapps/frontend/src/main/index.tsapps/frontend/src/renderer/lib/__tests__/actionScoring.test.tsapps/frontend/src/main/api/startup.tsapps/frontend/src/main/api/__tests__/websocket.spec.tsapps/frontend/src/main/api/schemas/index.tsapps/frontend/src/main/api/index.tsapps/frontend/src/renderer/lib/actionScoring.tsapps/frontend/src/main/api/routes/monitoring.tsapps/frontend/src/renderer/components/task-detail/TaskSubtasks.tsx
apps/frontend/**/*.{ts,tsx}
⚙️ CodeRabbit configuration file
apps/frontend/**/*.{ts,tsx}: Review React patterns and TypeScript type safety.
Check for proper state management and component composition.
Files:
apps/frontend/src/main/api/middleware/__tests__/auth.spec.tsapps/frontend/src/main/api/routes/tasks.tsapps/frontend/src/renderer/components/task-detail/TaskDetailModal.tsxapps/frontend/src/renderer/components/task-detail/SubtaskActionList.tsxapps/frontend/src/__tests__/integration/api-ipc-bridge.test.tsapps/frontend/src/main/api/server.tsapps/frontend/src/main/api/event-bridge.tsapps/frontend/src/main/api/types/index.tsapps/frontend/src/main/api/routes/projects.tsapps/frontend/src/main/api/routes/__tests__/tasks.spec.tsapps/frontend/src/__tests__/integration/openapi-spec.test.tsapps/frontend/src/main/api/routes/index.tsapps/frontend/src/main/api/middleware/auth.tsapps/frontend/src/main/api/websocket.tsapps/frontend/src/main/index.tsapps/frontend/src/renderer/lib/__tests__/actionScoring.test.tsapps/frontend/src/main/api/startup.tsapps/frontend/src/main/api/__tests__/websocket.spec.tsapps/frontend/src/main/api/schemas/index.tsapps/frontend/src/main/api/index.tsapps/frontend/src/renderer/lib/actionScoring.tsapps/frontend/src/main/api/routes/monitoring.tsapps/frontend/src/renderer/components/task-detail/TaskSubtasks.tsx
🧠 Learnings (3)
📓 Common learnings
Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-25T18:29:32.954Z
Learning: Applies to apps/backend/agents/{qa_reviewer,qa_fixer}.py : For frontend bug fixes and feature implementations, use the Electron MCP server for automated E2E testing when the app is running with `--remote-debugging-port=9222`
📚 Learning: 2025-12-25T18:29:32.954Z
Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-25T18:29:32.954Z
Learning: Applies to apps/backend/.env* : Enable Electron MCP in `apps/backend/.env` with `ELECTRON_MCP_ENABLED=true` and `ELECTRON_DEBUG_PORT=9222` to allow QA agents to interact with running Electron app
Applied to files:
apps/frontend/.env.example
📚 Learning: 2025-12-25T18:29:32.954Z
Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-25T18:29:32.954Z
Learning: Applies to apps/backend/agents/{qa_reviewer,qa_fixer}.py : For frontend bug fixes and feature implementations, use the Electron MCP server for automated E2E testing when the app is running with `--remote-debugging-port=9222`
Applied to files:
apps/frontend/.env.example
🧬 Code graph analysis (8)
apps/frontend/src/main/api/routes/tasks.ts (2)
apps/frontend/src/main/project-store.ts (1)
projectStore(776-776)apps/frontend/src/shared/constants/config.ts (2)
getSpecsDir(100-103)AUTO_BUILD_PATHS(77-94)
apps/frontend/src/renderer/components/task-detail/TaskDetailModal.tsx (1)
apps/frontend/src/renderer/components/task-detail/TaskSubtasks.tsx (1)
TaskSubtasks(63-343)
apps/frontend/src/__tests__/integration/api-ipc-bridge.test.ts (1)
apps/frontend/src/main/project-store.ts (1)
projectStore(776-776)
apps/frontend/src/main/api/routes/projects.ts (1)
apps/frontend/src/main/project-store.ts (1)
projectStore(776-776)
apps/frontend/src/main/api/routes/__tests__/tasks.spec.ts (1)
apps/frontend/src/main/project-store.ts (1)
projectStore(776-776)
apps/frontend/src/main/api/startup.ts (1)
apps/frontend/src/main/api/index.ts (24)
ApiStartupOptions(129-129)ApiStartupResult(130-130)ApiShutdownResult(131-131)ApiServerStatus(132-132)initializeApiServer(119-119)isServerRunning(40-40)getServerInstance(43-43)createApiServer(36-36)registerRoutes(53-53)startApiServer(37-37)initializeEventBridge(105-105)shutdownApiServer(120-120)getServerUptime(41-41)getClientCount(87-87)getEventBridgeStats(108-108)closeAllConnections(96-96)shutdownEventBridge(106-106)stopApiServer(38-38)getApiServerStatus(121-121)isEventBridgeActive(107-107)isApiServerEnabled(122-122)areApiKeysLoaded(124-124)areKeysLoaded(70-70)getApiServerInstance(125-125)
apps/frontend/src/renderer/lib/actionScoring.ts (1)
apps/frontend/src/shared/types/task.ts (2)
TaskLogEntry(62-75)TaskLogEntryType(60-60)
apps/frontend/src/main/api/routes/monitoring.ts (1)
apps/frontend/src/main/project-store.ts (1)
projectStore(776-776)
🪛 Biome (2.1.2)
apps/frontend/src/renderer/components/task-detail/SubtaskActionList.tsx
[error] 141-148: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 198-205: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 279-282: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 447-450: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
apps/frontend/src/renderer/components/task-detail/TaskSubtasks.tsx
[error] 142-152: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 298-306: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
🪛 Gitleaks (8.30.0)
apps/frontend/src/main/api/middleware/__tests__/auth.spec.ts
[high] 183-183: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
[high] 193-193: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.
(generic-api-key)
🔇 Additional comments (35)
apps/frontend/src/renderer/lib/__tests__/actionScoring.test.ts (2)
593-636: Verify performance test thresholds are CI-friendly.The performance tests expect sub-100ms for 1000 actions and sub-500ms for 5000 actions. These thresholds may be too strict for CI environments, which typically have slower/shared resources than development machines.
Consider one of these approaches:
- Increase the thresholds by 2-3x to account for CI variability (e.g., 300ms and 1500ms)
- Use relative performance assertions instead of absolute timings
- Skip performance tests in CI with a flag like
skipInCIBased on learnings, if QA agents have access to the running app, you could also create E2E performance tests that measure real-world scoring performance with the Electron MCP server.
1-1188: LGTM! Comprehensive test coverage.The test suite is well-structured with excellent coverage of:
- Scoring weights and detection logic
- Filtering and tiebreaking behavior
- File extraction and summarization
- Subtask relevance calculations
- Edge cases (null, empty, missing data)
- Non-mutation guarantees
The tests are clear, use appropriate helpers, and validate both positive and negative cases.
apps/frontend/src/main/index.ts (1)
9-9: LGTM! API server lifecycle integration is solid.The API server initialization and shutdown are properly integrated into the Electron app lifecycle with:
- Conditional enablement check
- Proper error handling for both resolved and rejected promises
- Consistent logging
- Clean shutdown in
before-quithandlerAlso applies to: 185-267
apps/frontend/src/__tests__/integration/openapi-spec.test.ts (1)
1-655: LGTM! Excellent OpenAPI spec validation.This integration test provides comprehensive coverage of:
- OpenAPI spec structure and metadata
- All REST endpoints (tasks, projects, monitoring)
- Request/response schemas
- Security schemes
- Query and path parameters
- WebSocket endpoint (with appropriate handling of OpenAPI limitations)
- Spec accessibility and consistency
The mocking approach is clean and the test structure makes it easy to verify complete API surface coverage.
apps/frontend/src/renderer/components/task-detail/TaskDetailModal.tsx (1)
420-424: LGTM! Clean prop integration.The addition of
phaseLogsandonViewAllLogsprops toTaskSubtasksis properly typed and integrates cleanly with the existing component. The callback enables navigation from the Subtasks tab to the Logs tab, which is a good UX improvement.apps/frontend/src/main/api/routes/index.ts (1)
26-40: No action required. TheregisterWebSocketRoutefunction returnsvoid(not a Promise), making it synchronous and intentionally not awaitable. The registration order—usingawaitfor async operations and direct calls for synchronous ones—is correct as written.Likely an incorrect or invalid review comment.
apps/frontend/src/main/api/middleware/__tests__/auth.spec.ts (1)
1-597: LGTM! Comprehensive test coverage for auth middleware.The test suite thoroughly covers all authentication functions including edge cases for security (empty strings, whitespace, non-string inputs, fail-closed behavior). The Gitleaks warnings on lines 183 and 193 are false positives as these are test API keys (
valid-key-123) used in unit tests, not actual secrets.apps/frontend/src/main/api/routes/__tests__/tasks.spec.ts (1)
1-645: LGTM! Comprehensive unit test coverage for task routes.The test suite covers all task CRUD operations, lifecycle endpoints (start/stop/review), authentication middleware verification, and appropriate error handling scenarios. Good use of beforeEach/afterEach for test isolation and cleanup.
apps/frontend/src/main/api/routes/monitoring.ts (1)
142-259: LGTM! Well-structured monitoring endpoints.The route registration correctly separates public endpoints (
/api/health,/api/version) from authenticated endpoints (/api/status). The dynamic import for project-store avoids circular dependency issues and gracefully handles failures by marking the component asdegraded.apps/frontend/src/main/api/__tests__/websocket.spec.ts (1)
1-912: LGTM! Comprehensive WebSocket test coverage.The test suite thoroughly covers client management, broadcast functions, connection lifecycle, message processing (subscribe/unsubscribe/ping), subscription filtering, and edge cases (closed sockets, send errors, unique IDs). Good use of mock patterns and clear test organization.
apps/frontend/src/__tests__/integration/api-ipc-bridge.test.ts (1)
1-636: LGTM! Solid integration tests verifying HTTP API mirrors IPC functionality.The test suite comprehensively validates that the HTTP API provides equivalent functionality to IPC handlers, including project and task operations, error handling, monitoring endpoints, and authentication. Good verification that both layers use the same projectStore.
apps/frontend/src/main/api/routes/tasks.ts (3)
51-66: LGTM! Helper function for finding tasks across projects.The helper iterates all projects to find a task by ID or specId. While this is O(projects × tasks), it's acceptable for typical project counts and mirrors IPC handler behavior.
440-460: Deletion allowed without checking if task is running.The comment on lines 441-443 acknowledges that the IPC handler checks if a task is running before deletion, but the HTTP API skips this check. This could lead to inconsistent behavior between IPC and HTTP API, and potentially orphaned running processes.
Consider whether the HTTP API should also check
AgentManagerfor running tasks before allowing deletion, or if this is intentional behavior for the remote API use case. If intentional, the API documentation should clarify this difference.
141-287: LGTM! Task creation with comprehensive spec generation.The POST /api/tasks endpoint properly creates spec directories, generates implementation_plan.json, task_metadata.json, and requirements.json files, handles attached images with base64 decoding, and uses zero-padded spec IDs.
apps/frontend/src/main/api/event-bridge.ts (1)
210-263: LGTM! Well-structured event bridge initialization.The initialization properly prevents double-initialization, subscribes to all relevant events from AgentManager and FileWatcher, and registers cleanup functions for proper teardown. The cleanup array pattern ensures all listeners are removed during shutdown.
apps/frontend/src/renderer/components/task-detail/SubtaskActionList.tsx (1)
407-487: LGTM! Well-structured component with good performance optimizations.The SubtaskActionList component uses
useMemoappropriately for expensive operations (slicing and grouping actions), handles empty states gracefully, and provides a clear visual hierarchy with grouped actions and score badges.apps/frontend/src/renderer/components/task-detail/TaskSubtasks.tsx (1)
63-120: LGTM - Component logic and memoization are well-structured.The use of
useMemoforallEntries,relevanceScores, andsortedSubtasksis appropriate for avoiding unnecessary recomputation. TheuseCallbackforgetSubtaskActionscorrectly captures theallEntriesdependency while avoiding recreation on every render.apps/frontend/src/main/api/startup.ts (1)
277-341: LGTM - Graceful shutdown implementation.The shutdown sequence is well-designed:
- Captures statistics before shutdown
- Closes WebSocket connections first
- Shuts down event bridge
- Stops the Fastify server
- Clears module state
Each step has individual error handling to ensure partial failures don't prevent subsequent cleanup steps.
apps/frontend/src/main/api/types/index.ts (1)
1-362: LGTM - Comprehensive and well-organized type definitions.The types module provides a clean API contract surface with:
- Proper re-exports from shared types for consistency
- Complete coverage of REST API request/response types
- WebSocket message types with proper payloads
- Route parameters and pagination helpers for future extensibility
apps/frontend/src/main/api/server.ts (3)
46-55: Verify CORS default allows all origins.The default CORS configuration allows all origins (
['*']) whenAPI_CORS_ORIGINSis not set. While convenient for development, this could be a security concern in production deployments.Consider documenting this behavior prominently or defaulting to a more restrictive setting:
function parseCorsOrigins(): string[] { const envOrigins = process.env.API_CORS_ORIGINS; if (!envOrigins) { - return ['*']; // Allow all origins by default (configurable) + // SECURITY: In production, set API_CORS_ORIGINS to restrict access + // Default allows all origins for local development convenience + return ['*']; }
239-264: LGTM - Error handling is well-implemented.The error handler appropriately:
- Falls back to 500 for unknown errors
- Logs 5xx errors at error level, 4xx at warn level
- Returns consistent error response format
- Provides a meaningful not-found handler with request details
101-107:loadApiKeys()throws if API_KEY is not set; consider documenting callers' responsibility or refactoring to return a result.
createApiServer()callsloadApiKeys()which throws if API_KEY is missing. WhileinitializeApiServer()(startup.ts) protects this call with a pre-check and try-catch, the publiccreateAndStartApiServer()function does not check API_KEY before callingcreateApiServer(), leaving callers of that function unprotected. Either document that all callers must check API_KEY first, or refactorloadApiKeys()to return a validation result to centralize this check.apps/frontend/src/main/api/websocket.ts (1)
593-598: Type assertion is acceptable but consider typing improvement.The
anytype assertion is documented with a comment explaining the necessity. For a cleaner approach, consider using Fastify's proper WebSocket route typing:import type { FastifyInstance, RouteShorthandOptions } from 'fastify'; // Using proper generic typing if available from @fastify/websocket fastify.get('/ws', routeOptions as RouteShorthandOptions, wsHandler);However, the current approach works and the comment adequately explains the rationale.
apps/frontend/src/main/api/middleware/auth.ts (1)
60-90: LGTM - API key loading implementation.The key loading function properly:
- Validates environment variable presence
- Parses comma-separated keys
- Filters empty strings
- Throws clear error messages for misconfiguration
apps/frontend/src/main/api/schemas/index.ts (2)
39-95: LGTM - Enum definitions align with TypeScript types.The const arrays for enums (
taskStatusEnum,subtaskStatusEnum, etc.) correctly match the corresponding TypeScript types and provide runtime values for JSON Schema validation.
993-1014: LGTM - Public endpoints correctly exclude security.The health and version endpoint schemas properly set
security: []to indicate they don't require authentication, which is the correct pattern for monitoring endpoints.apps/frontend/src/main/api/index.ts (1)
1-281: LGTM - Well-organized module entry point.The barrel file effectively consolidates the API surface with:
- Clear section headers for discoverability
- Comprehensive re-exports of functions and types
- Helpful usage example in the module documentation
- Logical grouping (server lifecycle, routes, auth, websocket, etc.)
apps/frontend/src/renderer/lib/actionScoring.ts (8)
1-31: LGTM!Well-documented constants with clear cognitive science rationale. The
as constassertion ensures type safety for the scoring weights.
33-60: LGTM!Type definitions are well-structured. Using
indexfor tiebreaking ensures deterministic sorting.
100-181: LGTM!Detection functions are well-implemented with multiple strategies (type-based and content-based). Case-insensitive handling is consistent.
183-198: LGTM!The heuristic approach is reasonable. Consider that the
{1,4}extension limit may miss some file types (e.g.,.swift,.scala), but for a scoring heuristic this is acceptable.
241-251: LGTM!Clean implementation of novelty detection using a set for O(1) lookups.
298-305: Side effect:scoreActionmutatescontext.seenToolTypes.This makes scoring order-dependent. The current usage in
filterTopActionsprocesses sequentially, so this works correctly. However, if this function is ever used in parallel processing or with a shared context across multiple calls, results could be inconsistent.Consider documenting this behavior in the JSDoc or making the mutation explicit in the caller.
389-408: LGTM!Clean grouping implementation with sensible default for missing subphase.
581-829: LGTM!File extraction utilities are well-implemented with:
- Multiple path format support (direct, JSON, regex patterns)
- Proper deduplication by skipping
tool_endevents- Clear categorization of modified vs read-only files
- Sensible defaults and caps for UI display
apps/frontend/.env.example
Outdated
| # API key for authentication (required for remote access) | ||
| # Generate a secure random key for production use | ||
| # If not set, API authentication is disabled (local development only) | ||
| # API_KEY=your-secure-api-key-here |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Clarify API authentication requirements.
The comments state "API key for authentication (required for remote access)" and "If not set, API authentication is disabled (local development only)". This creates ambiguity about when authentication is actually enforced.
Consider clarifying the behavior:
🔎 Suggested clarification
-# API key for authentication (required for remote access)
-# Generate a secure random key for production use
-# If not set, API authentication is disabled (local development only)
+# API key for authentication
+# REQUIRED when API_HOST is not 127.0.0.1 (remote access)
+# OPTIONAL for localhost-only access (development)
+# Generate with: node -e "console.log(require('crypto').randomBytes(32).toString('hex'))"
# API_KEY=your-secure-api-key-hereAlso consider whether disabling authentication even for localhost is secure, given that malicious local software could access the API.
🤖 Prompt for AI Agents
In apps/frontend/.env.example lines 52-55, the existing comments about API_KEY
are ambiguous about when authentication is enforced; update the comment to (1)
explicitly state that setting API_KEY enables authentication for all requests,
(2) clarify that if API_KEY is empty the server will run in an
"allow-unauthenticated" mode used only for local development, and (3) add a
security note advising not to run without an API_KEY on any machine exposed to
untrusted software and recommending generating a strong secret for production
and/or using a separate flag or bind-address restriction to allow
unauthenticated localhost-only access. Ensure the new comment gives exact
instructions to set API_KEY for production and warns about localhost risk.
apps/frontend/package.json
Outdated
| "@fastify/cors": "^11.2.0", | ||
| "@fastify/swagger": "^9.4.2", | ||
| "@fastify/swagger-ui": "^5.2.2", | ||
| "@fastify/websocket": "^11.0.2", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check current versions of Fastify and related packages
echo "=== Current npm versions ==="
npm info fastify --json | jq '.["dist-tags"]'
npm info @fastify/cors --json | jq '.["dist-tags"]'
npm info @fastify/swagger --json | jq '.["dist-tags"]'
npm info @fastify/swagger-ui --json | jq '.["dist-tags"]'
npm info @fastify/websocket --json | jq '.["dist-tags"]'
echo -e "\n=== Version details for Fastify 5.3.4 ==="
npm info [email protected] --json | jq '{name: .name, version: .version, published: .time.created, latest: .time[.["dist-tags"]["latest"]]}'
echo -e "\n=== Plugin version details ==="
npm info @fastify/[email protected] --json | jq '{name: .name, version: .version, published: .time.created}'
npm info @fastify/[email protected] --json | jq '{name: .name, version: .version, published: .time.created}'Repository: AndyMik90/Auto-Claude
Length of output: 1066
🌐 Web query:
Fastify 5.x production-ready stability release notes compatibility
💡 Result:
Short answer
- Fastify 5 is a stable, production-ready major series (first stable tag v5.0.0 published Sep 17, 2024) and is actively maintained with 5.x patch/minor releases. [1][2]
Key compatibility / production notes
- Node.js requirement: v20+ only (v5 drops support for older Node versions). [1]
- Many breaking changes vs v4 (full JSON Schema required for query/params/body/response, changed .listen() behavior, removed deprecated APIs, changed logger signature, parameters object prototype removed, type-provider split, plugins must not mix callback/promise APIs, etc.). See the full migration/breaking-changes list. [1]
- Plugin ecosystem: many community plugins required updated fastify-plugin peer ranges and code changes; check individual plugins for v5 support before upgrading. See the tracking issue and plugin updates. [3]
- Security/patches: v5 receives regular security/bug patches (see GitHub releases/changelog for v5.x). [2]
Upgrade checklist (brief)
- Upgrade Node to v20+.
- Fix all v4 deprecation warnings first.
- Read and apply the Migration Guide (address schema changes, new APIs, removed APIs).
- Verify each plugin you use is updated for fastify@^5 (or replace/patch it).
- Run full test suite and smoke-test in staging before production deploy.
Links
- Migration guide / breaking changes (Fastify v5 docs). [1]
- Fastify releases / changelog (GitHub releases). [2]
- Plugin compatibility tracking issue (plugins needing updates for v5). [3]
References
[1] Fastify Migration Guide V5 (breaking changes & requirements).
[2] fastify/fastify Releases (v5.x changelog).
[3] GitHub issue tracking plugins needing updates for Fastify v5.
🌐 Web query:
@fastify/cors @fastify/swagger @fastify/websocket Fastify 5.x compatibility
💡 Result:
Short answer — yes; use the plugins' v5-compatible releases:
- @fastify/cors: use v^10 / v^11 (compat with Fastify ^5.x). [1]
- @fastify/swagger: use v9.x (>=9.x compatible with Fastify ^5.x). [2]
- @fastify/swagger-ui: use v^5.x (compatible with Fastify ^5.x; pairs with @fastify/swagger v9). [3]
- @fastify/websocket: use the @fastify/websocket package (v5.x series for Fastify 5). [4]
References:
[1] @fastify/cors compatibility table.
[2] @fastify/swagger compatibility table / README.
[3] @fastify/swagger-ui README (version → Fastify mapping).
[4] @fastify/websocket README / repo (maintained @fastify/websocket package).
🌐 Web query:
Fastify security vulnerabilities CVE 2024 2025
💡 Result:
Here are notable Fastify-related CVEs from 2024–2025 (summary + patched versions and dates):
-
CVE-2024-35220 — @fastify/session: cookie expiration/maxAge bug that could keep sessions alive. Patched in @fastify/session 10.8.0 (disclosed 2024). [1]
-
CVE-2024-31999 — @fastify/secure-session: deleted/encrypted session cookies could remain reusable. Patched in @fastify/secure-session 7.3.0 (disclosed Apr 10, 2024). [2]
-
CVE-2024-22207 — @fastify/swagger-ui: default config could expose module files when baseDir not set. Affected @fastify/swagger-ui 2.0.0–2.1.0; patched (disclosed Jan 15, 2024). [3]
-
CVE-2025-24033 — @fastify/multipart: temporary uploaded files not deleted when requests are cancelled (resource exhaustion/cleanup). Patched in 8.3.1 and 9.0.3 (disclosed Jan 23, 2025). [4]
-
CVE-2025-32442 — fastify core: content-type validation bypass for slightly altered content-types (affects fastify >=5.0.0 <5.3.2 and specific 4.x). Patched in fastify 5.3.2 and 4.9.1 (published Apr 18, 2025). [5]
-
CVE-2025-66415 — fastify-reply-from: crafted URLs could access routes not intended to be proxied. Patched in fastify-reply-from 12.5.0 (2025). [6]
Recommended actions (brief):
- Upgrade the listed packages to the patched versions above (or latest). [1][2][3][4][5][6]
- Audit your dependency tree (npm audit / SCA) and rebuild images/containers after upgrades.
- If you cannot immediately upgrade, apply vendor workarounds noted in advisories (see linked advisories for each CVE).
Sources:
[1] NVD / @fastify/session advisory (CVE-2024-35220). (nvd.nist.gov)
[2] Wiz / GitHub advisory summary (CVE-2024-31999). (wiz.io)
[3] Wiz / GitHub advisory (CVE-2024-22207). (wiz.io)
[4] Wiz vulnerability DB (CVE-2025-24033). (wiz.io)
[5] cve.circl / GitHub advisory (CVE-2025-32442). (cve.circl.lu)
[6] NVD record for CVE-2025-66415 (fastify-reply-from). (nvd.nist.gov)
If you’d like, I can fetch the exact advisory/commit links and suggested mitigations for any specific CVE above.
Citations:
- 1: https://nvd.nist.gov/vuln/detail/CVE-2024-35220?utm_source=openai
- 2: https://www.wiz.io/vulnerability-database/cve/cve-2024-31999?utm_source=openai
- 3: https://www.wiz.io/vulnerability-database/cve/cve-2024-22207?utm_source=openai
- 4: https://www.wiz.io/vulnerability-database/cve/cve-2025-24033?utm_source=openai
- 5: https://cve.circl.lu/vuln/cve-2025-32442?utm_source=openai
- 6: https://nvd.nist.gov/vuln/detail/CVE-2025-66415?utm_source=openai
[email protected] does not exist in npm registry—this must be corrected.
The version [email protected] returns a 404 error. The latest stable 5.x release is 5.6.2. Update the dependency to a valid version (e.g., ^5.6.2). Note that fastify 5 requires Node.js v20+ and includes breaking changes from v4; ensure the codebase accommodates the migration (full JSON Schema, .listen() behavior, logger signature changes, etc.).
All plugin versions (@fastify/cors ^11.2.0, @fastify/swagger ^9.4.2, @fastify/swagger-ui ^5.2.2, @fastify/websocket ^11.0.2) are compatible with Fastify 5.x. No known security vulnerabilities exist in these plugin versions.
🤖 Prompt for AI Agents
In apps/frontend/package.json around lines 53 to 56, the fastify dependency is
pinned to a non-existent version (5.3.4); update the fastify entry to a valid
release such as "^5.6.2", run npm/yarn install to update lockfile, and run the
test suite; additionally ensure the project uses Node.js v20+ and audit/adjust
code for Fastify v5 breaking changes (full JSON Schema usage, .listen() async
signature and return handling, and any logger signature differences) so runtime
errors do not occur after the upgrade.
| function handleAgentExit(taskId: string, code: number | null, processType: ProcessType): void { | ||
| state.eventCount++; | ||
|
|
||
| // Determine new status based on process type and exit code | ||
| // This mirrors the logic in agent-events-handlers.ts | ||
| let newStatus: TaskStatus; | ||
|
|
||
| if (processType === 'spec-creation') { | ||
| // Spec creation doesn't trigger a status change via WebSocket | ||
| // The status is updated through other means | ||
| if (debugMode) { | ||
| process.stdout.write(`[EventBridge] exit (spec-creation): ${taskId} code=${code}\n`); | ||
| } | ||
| return; | ||
| } | ||
|
|
||
| if (processType === 'task-execution' || processType === 'qa-process') { | ||
| // Task execution or QA process completed -> Human Review | ||
| newStatus = 'human_review'; | ||
| } else { | ||
| // Unknown process type -> Human Review | ||
| newStatus = 'human_review'; | ||
| } | ||
|
|
||
| if (debugMode) { | ||
| process.stdout.write( | ||
| `[EventBridge] exit: ${taskId} code=${code} type=${processType} -> ${newStatus}\n` | ||
| ); | ||
| } | ||
|
|
||
| broadcastTaskStatusChange(taskId, newStatus); | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Redundant else branch in handleAgentExit.
Lines 120-126 have a redundant else branch - both the if and else conditions return 'human_review'. This appears to be placeholder code or incomplete logic.
🔎 Proposed simplification
if (processType === 'task-execution' || processType === 'qa-process') {
// Task execution or QA process completed -> Human Review
newStatus = 'human_review';
- } else {
- // Unknown process type -> Human Review
- newStatus = 'human_review';
}
+ // Default: Human Review (for unknown process types)
+ newStatus = 'human_review';Or if different status mappings are intended for future process types, consider using a switch statement with explicit cases.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| function handleAgentExit(taskId: string, code: number | null, processType: ProcessType): void { | |
| state.eventCount++; | |
| // Determine new status based on process type and exit code | |
| // This mirrors the logic in agent-events-handlers.ts | |
| let newStatus: TaskStatus; | |
| if (processType === 'spec-creation') { | |
| // Spec creation doesn't trigger a status change via WebSocket | |
| // The status is updated through other means | |
| if (debugMode) { | |
| process.stdout.write(`[EventBridge] exit (spec-creation): ${taskId} code=${code}\n`); | |
| } | |
| return; | |
| } | |
| if (processType === 'task-execution' || processType === 'qa-process') { | |
| // Task execution or QA process completed -> Human Review | |
| newStatus = 'human_review'; | |
| } else { | |
| // Unknown process type -> Human Review | |
| newStatus = 'human_review'; | |
| } | |
| if (debugMode) { | |
| process.stdout.write( | |
| `[EventBridge] exit: ${taskId} code=${code} type=${processType} -> ${newStatus}\n` | |
| ); | |
| } | |
| broadcastTaskStatusChange(taskId, newStatus); | |
| } | |
| function handleAgentExit(taskId: string, code: number | null, processType: ProcessType): void { | |
| state.eventCount++; | |
| // Determine new status based on process type and exit code | |
| // This mirrors the logic in agent-events-handlers.ts | |
| let newStatus: TaskStatus; | |
| if (processType === 'spec-creation') { | |
| // Spec creation doesn't trigger a status change via WebSocket | |
| // The status is updated through other means | |
| if (debugMode) { | |
| process.stdout.write(`[EventBridge] exit (spec-creation): ${taskId} code=${code}\n`); | |
| } | |
| return; | |
| } | |
| if (processType === 'task-execution' || processType === 'qa-process') { | |
| // Task execution or QA process completed -> Human Review | |
| newStatus = 'human_review'; | |
| } | |
| // Default: Human Review (for unknown process types) | |
| newStatus = 'human_review'; | |
| if (debugMode) { | |
| process.stdout.write( | |
| `[EventBridge] exit: ${taskId} code=${code} type=${processType} -> ${newStatus}\n` | |
| ); | |
| } | |
| broadcastTaskStatusChange(taskId, newStatus); | |
| } |
🤖 Prompt for AI Agents
In apps/frontend/src/main/api/event-bridge.ts around lines 104 to 135, the
if/else that sets newStatus is redundant because both branches assign
'human_review'; simplify by removing the unnecessary else branch and directly
assigning newStatus = 'human_review' for the known process types (or replace the
whole conditional with a switch or a mapping object if you plan to support
additional processType values), ensure debug logging and
broadcastTaskStatusChange(taskId, newStatus) remain unchanged.
| export function validateApiKey(apiKey: string | undefined | null): ApiKeyValidation { | ||
| if (!apiKey || typeof apiKey !== 'string') { | ||
| return { valid: false }; | ||
| } | ||
|
|
||
| if (!keysLoaded) { | ||
| // Keys not loaded - fail closed (reject) | ||
| return { valid: false }; | ||
| } | ||
|
|
||
| const isValid = authConfig.validKeys.has(apiKey); | ||
|
|
||
| return { | ||
| valid: isValid, | ||
| // Note: We don't include keyId to avoid leaking information | ||
| }; | ||
| } |
There was a problem hiding this comment.
Potential timing attack vulnerability in key validation.
Using Set.has() for key comparison may be vulnerable to timing attacks, as Set lookups can have variable timing based on the key's hash bucket location.
For API key validation, consider using a constant-time comparison:
Suggested improvement
import { timingSafeEqual } from 'crypto';
export function validateApiKey(apiKey: string | undefined | null): ApiKeyValidation {
if (!apiKey || typeof apiKey !== 'string' || !keysLoaded) {
return { valid: false };
}
// Constant-time comparison against all valid keys
const apiKeyBuffer = Buffer.from(apiKey);
let isValid = false;
for (const validKey of authConfig.validKeys) {
const validKeyBuffer = Buffer.from(validKey);
if (apiKeyBuffer.length === validKeyBuffer.length &&
timingSafeEqual(apiKeyBuffer, validKeyBuffer)) {
isValid = true;
// Don't break early to maintain constant time
}
}
return { valid: isValid };
}🤖 Prompt for AI Agents
In apps/frontend/src/main/api/middleware/auth.ts around lines 113 to 129, the
current Set.has() lookup can leak timing information; replace it with a
constant-time comparison: import crypto.timingSafeEqual, convert the incoming
apiKey and each stored key to Buffers, iterate over all stored keys comparing
lengths and using timingSafeEqual (do not break early) and accumulate a boolean
match flag, and ensure the early checks (apiKey type and keysLoaded) remain but
return only the accumulated result to avoid timing differences.
| export function resetAuthState(): void { | ||
| authConfig = { ...defaultAuthConfig }; | ||
| keysLoaded = false; | ||
| } | ||
|
|
||
| /** | ||
| * Configure auth with custom keys (for testing purposes only) | ||
| * | ||
| * This function allows setting API keys programmatically for tests. | ||
| * In production, always use loadApiKeys() to load from environment. | ||
| * | ||
| * @param keys - Array of valid API keys | ||
| */ | ||
| export function setApiKeysForTesting(keys: string[]): void { | ||
| authConfig = { | ||
| ...defaultAuthConfig, | ||
| validKeys: new Set(keys), | ||
| }; | ||
| keysLoaded = true; | ||
| } |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Testing utilities are exported from production code.
resetAuthState() and setApiKeysForTesting() are exported and could be accidentally used in production code. Consider:
- Moving these to a separate test utilities file
- Adding runtime checks to prevent production usage:
export function setApiKeysForTesting(keys: string[]): void {
+ if (process.env.NODE_ENV === 'production') {
+ throw new Error('setApiKeysForTesting cannot be used in production');
+ }
authConfig = {
...defaultAuthConfig,
validKeys: new Set(keys),
};
keysLoaded = true;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| export function resetAuthState(): void { | |
| authConfig = { ...defaultAuthConfig }; | |
| keysLoaded = false; | |
| } | |
| /** | |
| * Configure auth with custom keys (for testing purposes only) | |
| * | |
| * This function allows setting API keys programmatically for tests. | |
| * In production, always use loadApiKeys() to load from environment. | |
| * | |
| * @param keys - Array of valid API keys | |
| */ | |
| export function setApiKeysForTesting(keys: string[]): void { | |
| authConfig = { | |
| ...defaultAuthConfig, | |
| validKeys: new Set(keys), | |
| }; | |
| keysLoaded = true; | |
| } | |
| export function resetAuthState(): void { | |
| authConfig = { ...defaultAuthConfig }; | |
| keysLoaded = false; | |
| } | |
| /** | |
| * Configure auth with custom keys (for testing purposes only) | |
| * | |
| * This function allows setting API keys programmatically for tests. | |
| * In production, always use loadApiKeys() to load from environment. | |
| * | |
| * @param keys - Array of valid API keys | |
| */ | |
| export function setApiKeysForTesting(keys: string[]): void { | |
| if (process.env.NODE_ENV === 'production') { | |
| throw new Error('setApiKeysForTesting cannot be used in production'); | |
| } | |
| authConfig = { | |
| ...defaultAuthConfig, | |
| validKeys: new Set(keys), | |
| }; | |
| keysLoaded = true; | |
| } |
MikeeBuilds
added
feature
kvnloo
force-pushed
the
feat/enhanceSubtaskView
branch
from
ec7c63c to
ed006cd
Compare
|
@AndyMik90 sorry about that, I didn't realize the REST API commits got included here. I just fixed it with a git cherry-pick, it should be good now! |
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (13)
apps/frontend/src/renderer/components/task-detail/SubtaskActionList.tsx (5)
141-161: Add explicittype="button"to prevent unintended form submissions.This button lacks an explicit type attribute. If used inside a form, it would default to
submitand could trigger unintended form submissions.🔎 Proposed fix
<button + type="button" onClick={() => setIsExpanded(!isExpanded)} className={cn(
198-208: Add explicittype="button"to this button as well.Same issue - missing explicit type attribute.
🔎 Proposed fix
<button + type="button" onClick={() => setIsExpanded(!isExpanded)}
279-295: Add explicittype="button"to the CollapsibleTrigger button.🔎 Proposed fix
<button + type="button" className={cn( 'w-full flex items-center justify-between px-2 py-1.5 rounded-md transition-colors',
447-453: Add explicittype="button"to the "View all logs" button.🔎 Proposed fix
<button + type="button" onClick={onViewAllLogs}
426-453: Hardcoded user-facing strings should use translation keys.Per coding guidelines, all user-facing text in React components should use
useTranslation()with translation keys. The following strings need internationalization:
- Line 430:
"No actions recorded"- Line 444:
"Top {n} of {m} actions"- Line 451:
"View all logs"- Line 482:
"Showing top {n} most relevant from {m} total actions"Additionally, strings in
FilesSection(lines 339, 347, 374) andgetToolInfolabels should be internationalized.As per coding guidelines, consider using the
useTranslationhook:import { useTranslation } from 'react-i18next'; // In component: const { t } = useTranslation('taskDetail'); // Usage: <p>{t('subtaskActions.noActionsRecorded')}</p> <span>{t('subtaskActions.topActions', { displayed: displayActions.length, total: actions.length })}</span>apps/frontend/src/renderer/components/task-detail/TaskSubtasks.tsx (3)
2-2: Remove unused importArrowUpDown.The
ArrowUpDownicon is imported but never used in this file.-import { CheckCircle2, Clock, XCircle, AlertCircle, ListChecks, FileCode, ChevronDown, ChevronRight, Sparkles, ArrowUpDown, ArrowDownWideNarrow, ListOrdered } from 'lucide-react'; +import { CheckCircle2, Clock, XCircle, AlertCircle, ListChecks, FileCode, ChevronDown, ChevronRight, Sparkles, ArrowDownWideNarrow, ListOrdered } from 'lucide-react';
142-161: Add explicittype="button"to prevent unintended form submission.Per static analysis, buttons without an explicit type default to
submit.<button + type="button" onClick={toggleSortMode} className={cn(
298-314: Add explicittype="button"to the CollapsibleTrigger button.<button + type="button" className={cn(apps/frontend/src/renderer/lib/actionScoring.ts (5)
95-98: Remove unused constantSUCCESS_TYPES.This constant is defined but never referenced anywhere in the file.
🔎 Proposed fix
-/** - * Entry types that indicate success (lower priority than errors) - */ -const SUCCESS_TYPES: TaskLogEntryType[] = ['success'];
200-239: Time anomaly scoring is effectively disabled.
parseDurationalways returnsundefined, which means:
calculateAverageDurationalways returnsundefinedisTimeAnomalyalways returnsfalse- The
TIME_ANOMALYweight (10 points) is never appliedConsider adding a TODO comment to clarify intent, or removing the dead code path until duration tracking is implemented:
// TODO: Implement duration-based scoring when TaskLogEntry includes duration data
576-578:indexOfin sort comparator is O(n) per comparison.Using
subtaskIds.indexOf(a)inside the sort comparator performs a linear search for each comparison, leading to O(n² log n) complexity for large arrays.🔎 Proposed fix using index map
export function sortSubtasksByRelevance( subtaskIds: string[], relevanceScores: Map<string, SubtaskRelevanceScore> ): string[] { + // Pre-compute index map for O(1) lookups + const indexMap = new Map(subtaskIds.map((id, idx) => [id, idx])); + return [...subtaskIds].sort((a, b) => { // ... existing logic ... // Tertiary: stable sort by original index - return subtaskIds.indexOf(a) - subtaskIds.indexOf(b); + return (indexMap.get(a) ?? 0) - (indexMap.get(b) ?? 0); }); }
344-387: Consider consolidating duplicate scoring logic.The two branches (lines 360-369 for small arrays, lines 371-386 for larger) contain nearly identical code. The early return for small arrays doesn't provide meaningful optimization.
🔎 Proposed simplification
export function filterTopActions( actions: TaskLogEntry[], n: number = DEFAULT_TOP_N, subtaskId?: string ): ScoredAction[] { - if (!actions || actions.length === 0) { + if (!actions || actions.length === 0 || n <= 0) { return []; } let filteredActions = subtaskId ? actions.filter((a) => a.subtask_id === subtaskId) : actions; - if (filteredActions.length <= n) { - const context: ScoringContext = { ... }; - return filteredActions - .map((action, index) => scoreAction(action, index, context)) - .sort(compareScoredActions); - } - const context: ScoringContext = { averageDuration: calculateAverageDuration(filteredActions), seenToolTypes: new Set<string>(), }; const scoredActions = filteredActions.map((action, index) => scoreAction(action, index, context) ); scoredActions.sort(compareScoredActions); - return scoredActions.slice(0, n); + return scoredActions.slice(0, Math.min(n, scoredActions.length)); }
416-445: Hardcoded user-facing strings should use translation keys.Per coding guidelines,
getScoreReasonreturns hardcoded English strings that appear to be displayed in the UI (viaScoreBadgecomponent). These should be internationalized.Consider returning translation keys instead of hardcoded strings:
export const SCORE_REASON_KEYS = { ERROR: 'actionScoring:reasons.errorDetected', WARNING: 'actionScoring:reasons.warningDetected', DECISION: 'actionScoring:reasons.keyDecision', // ... } as const;Then have UI components translate the keys using
useTranslation(). As per coding guidelines, user-facing text must use translation keys.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (5)
apps/frontend/src/renderer/components/task-detail/SubtaskActionList.tsxapps/frontend/src/renderer/components/task-detail/TaskDetailModal.tsxapps/frontend/src/renderer/components/task-detail/TaskSubtasks.tsxapps/frontend/src/renderer/lib/__tests__/actionScoring.test.tsapps/frontend/src/renderer/lib/actionScoring.ts
🧰 Additional context used
📓 Path-based instructions (2)
apps/frontend/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/frontend/src/**/*.{ts,tsx}: Always use translation keys withuseTranslation()for all user-facing text in React/TypeScript frontend components - use formatnamespace:section.key(e.g.,navigation:items.githubPRs)
Never use hardcoded strings in JSX/TSX files for user-facing text - always reference translation keys fromapps/frontend/src/shared/i18n/locales/
Files:
apps/frontend/src/renderer/components/task-detail/TaskDetailModal.tsxapps/frontend/src/renderer/components/task-detail/TaskSubtasks.tsxapps/frontend/src/renderer/lib/actionScoring.tsapps/frontend/src/renderer/lib/__tests__/actionScoring.test.tsapps/frontend/src/renderer/components/task-detail/SubtaskActionList.tsx
apps/frontend/**/*.{ts,tsx}
⚙️ CodeRabbit configuration file
apps/frontend/**/*.{ts,tsx}: Review React patterns and TypeScript type safety.
Check for proper state management and component composition.
Files:
apps/frontend/src/renderer/components/task-detail/TaskDetailModal.tsxapps/frontend/src/renderer/components/task-detail/TaskSubtasks.tsxapps/frontend/src/renderer/lib/actionScoring.tsapps/frontend/src/renderer/lib/__tests__/actionScoring.test.tsapps/frontend/src/renderer/components/task-detail/SubtaskActionList.tsx
🧠 Learnings (3)
📚 Learning: 2025-12-25T18:29:32.954Z
Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-25T18:29:32.954Z
Learning: Applies to apps/frontend/src/**/*.{ts,tsx} : Always use translation keys with `useTranslation()` for all user-facing text in React/TypeScript frontend components - use format `namespace:section.key` (e.g., `navigation:items.githubPRs`)
Applied to files:
apps/frontend/src/renderer/lib/actionScoring.tsapps/frontend/src/renderer/components/task-detail/SubtaskActionList.tsx
📚 Learning: 2025-12-25T18:29:32.954Z
Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-25T18:29:32.954Z
Learning: Applies to apps/frontend/src/**/*.{ts,tsx} : Never use hardcoded strings in JSX/TSX files for user-facing text - always reference translation keys from `apps/frontend/src/shared/i18n/locales/`
Applied to files:
apps/frontend/src/renderer/lib/actionScoring.tsapps/frontend/src/renderer/components/task-detail/SubtaskActionList.tsx
📚 Learning: 2025-12-25T18:29:32.954Z
Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-25T18:29:32.954Z
Learning: Applies to apps/frontend/src/shared/i18n/locales/**/*.json : Add new translation keys to ALL language files (minimum: `en/*.json` and `fr/*.json`) in `apps/frontend/src/shared/i18n/locales/`
Applied to files:
apps/frontend/src/renderer/components/task-detail/SubtaskActionList.tsx
🧬 Code graph analysis (3)
apps/frontend/src/renderer/components/task-detail/TaskDetailModal.tsx (1)
apps/frontend/src/renderer/components/task-detail/TaskSubtasks.tsx (1)
TaskSubtasks(63-343)
apps/frontend/src/renderer/components/task-detail/TaskSubtasks.tsx (3)
apps/frontend/src/shared/types/task.ts (3)
Task(235-255)TaskLogs(87-96)TaskLogEntry(64-77)apps/frontend/src/renderer/lib/actionScoring.ts (4)
filterTopActions(344-387)calculateSubtaskRelevanceScores(524-535)sortSubtasksByRelevance(548-579)getImportantFiles(814-829)apps/frontend/src/renderer/components/task-detail/SubtaskActionList.tsx (1)
SubtaskActionList(407-487)
apps/frontend/src/renderer/lib/actionScoring.ts (1)
apps/frontend/src/shared/types/task.ts (2)
TaskLogEntry(64-77)TaskLogEntryType(62-62)
🪛 Biome (2.1.2)
apps/frontend/src/renderer/components/task-detail/TaskSubtasks.tsx
[error] 142-152: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 298-306: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
apps/frontend/src/renderer/components/task-detail/SubtaskActionList.tsx
[error] 141-148: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 198-205: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 279-282: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
[error] 447-450: Provide an explicit type prop for the button element.
The default type of a button is submit, which causes the submission of a form when placed inside a form element. This is likely not the behaviour that you want inside a React application.
Allowed button types are: submit, button or reset
(lint/a11y/useButtonType)
🔇 Additional comments (8)
apps/frontend/src/renderer/components/task-detail/TaskDetailModal.tsx (1)
426-430: LGTM!The integration cleanly passes
phaseLogsfrom the modal's state and provides anonViewAllLogscallback to navigate to the logs tab. This properly connects the new action summarization UI to the existing modal architecture.apps/frontend/src/renderer/lib/__tests__/actionScoring.test.ts (2)
1-52: Well-structured test file with comprehensive coverage.The test helpers
createTestActionandcreateScoringContextare cleanly factored, and the file header clearly documents the test scope. The coverage spans constants, predicates, scoring logic, filtering, grouping, relevance scoring, and file extraction.
858-870: Good defensive test for immutability.The test verifying
sortSubtasksByRelevancedoesn't mutate the original array is a valuable safeguard against accidental side effects.apps/frontend/src/renderer/components/task-detail/SubtaskActionList.tsx (1)
48-65: LGTM - Tool info helper is well-structured.The
getToolInfofunction provides consistent styling and icon mapping for different tool types, mirroring the pattern fromTaskLogs.tsxas noted in the comment.apps/frontend/src/renderer/components/task-detail/TaskSubtasks.tsx (2)
36-61: LGTM - Helper functions are cleanly factored.
getAllEntriesFromPhaseLogsandgetSubtaskActionCountare well-structured utilities that centralize phase log traversal logic. The iteration over all three phases ensures comprehensive entry collection.
90-100: Good memoization strategy for scored actions.Using
useCallbackwithallEntriesdependency forgetSubtaskActionsensures scored actions are only recomputed when the underlying entries change, not on every render. The lazy computation pattern (only when expanded) is a good performance optimization.apps/frontend/src/renderer/lib/actionScoring.ts (2)
1-28: Well-documented module with clear cognitive science rationale.The module header effectively explains the scoring weights with references to relevant research (Csikszentmihalyi, Kahneman, Bjork). The exported constants and types provide a clean public API for the scoring system.
472-515: LGTM - Subtask relevance scoring is well-designed.The
calculateSubtaskRelevancefunction correctly aggregates individual action scores into a composite subtask relevance score. The flags forhasErrorsandhasDecisionsenable effective prioritization in the UI.
apps/frontend/src/renderer/components/task-detail/SubtaskActionList.tsx
Outdated
Show resolved
Hide resolved
apps/frontend/src/renderer/components/task-detail/TaskSubtasks.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
apps/frontend/src/renderer/components/task-detail/TaskDetailModal.tsx
🧰 Additional context used
📓 Path-based instructions (2)
apps/frontend/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/frontend/src/**/*.{ts,tsx}: Always use translation keys withuseTranslation()for all user-facing text in React/TypeScript frontend components - use formatnamespace:section.key(e.g.,navigation:items.githubPRs)
Never use hardcoded strings in JSX/TSX files for user-facing text - always reference translation keys fromapps/frontend/src/shared/i18n/locales/
Files:
apps/frontend/src/renderer/components/task-detail/TaskDetailModal.tsx
apps/frontend/**/*.{ts,tsx}
⚙️ CodeRabbit configuration file
apps/frontend/**/*.{ts,tsx}: Review React patterns and TypeScript type safety.
Check for proper state management and component composition.
Files:
apps/frontend/src/renderer/components/task-detail/TaskDetailModal.tsx
🧬 Code graph analysis (1)
apps/frontend/src/renderer/components/task-detail/TaskDetailModal.tsx (1)
apps/frontend/src/renderer/components/task-detail/TaskSubtasks.tsx (1)
TaskSubtasks(63-343)
There was a problem hiding this comment.
Actionable comments posted: 6
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (7)
apps/frontend/src/renderer/components/task-detail/SubtaskActionList.tsxapps/frontend/src/renderer/components/task-detail/TaskDetailModal.tsxapps/frontend/src/renderer/components/task-detail/TaskSubtasks.tsxapps/frontend/src/renderer/lib/__tests__/actionScoring.test.tsapps/frontend/src/renderer/lib/actionScoring.tsapps/frontend/src/shared/i18n/locales/en/tasks.jsonapps/frontend/src/shared/i18n/locales/fr/tasks.json
🧰 Additional context used
📓 Path-based instructions (3)
apps/frontend/src/shared/i18n/locales/**/*.json
📄 CodeRabbit inference engine (CLAUDE.md)
Add new translation keys to ALL language files (minimum:
en/*.jsonandfr/*.json) inapps/frontend/src/shared/i18n/locales/
Files:
apps/frontend/src/shared/i18n/locales/fr/tasks.jsonapps/frontend/src/shared/i18n/locales/en/tasks.json
apps/frontend/src/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/frontend/src/**/*.{ts,tsx}: Always use translation keys withuseTranslation()for all user-facing text in React/TypeScript frontend components - use formatnamespace:section.key(e.g.,navigation:items.githubPRs)
Never use hardcoded strings in JSX/TSX files for user-facing text - always reference translation keys fromapps/frontend/src/shared/i18n/locales/
Files:
apps/frontend/src/renderer/components/task-detail/TaskDetailModal.tsxapps/frontend/src/renderer/lib/__tests__/actionScoring.test.tsapps/frontend/src/renderer/components/task-detail/SubtaskActionList.tsxapps/frontend/src/renderer/components/task-detail/TaskSubtasks.tsxapps/frontend/src/renderer/lib/actionScoring.ts
apps/frontend/**/*.{ts,tsx}
⚙️ CodeRabbit configuration file
apps/frontend/**/*.{ts,tsx}: Review React patterns and TypeScript type safety.
Check for proper state management and component composition.
Files:
apps/frontend/src/renderer/components/task-detail/TaskDetailModal.tsxapps/frontend/src/renderer/lib/__tests__/actionScoring.test.tsapps/frontend/src/renderer/components/task-detail/SubtaskActionList.tsxapps/frontend/src/renderer/components/task-detail/TaskSubtasks.tsxapps/frontend/src/renderer/lib/actionScoring.ts
🧠 Learnings (3)
📚 Learning: 2025-12-25T18:29:32.954Z
Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-25T18:29:32.954Z
Learning: Applies to apps/frontend/src/shared/i18n/locales/**/*.json : Add new translation keys to ALL language files (minimum: `en/*.json` and `fr/*.json`) in `apps/frontend/src/shared/i18n/locales/`
Applied to files:
apps/frontend/src/shared/i18n/locales/fr/tasks.jsonapps/frontend/src/shared/i18n/locales/en/tasks.jsonapps/frontend/src/renderer/components/task-detail/SubtaskActionList.tsx
📚 Learning: 2025-12-25T18:29:32.954Z
Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-25T18:29:32.954Z
Learning: Applies to apps/frontend/src/**/*.{ts,tsx} : Never use hardcoded strings in JSX/TSX files for user-facing text - always reference translation keys from `apps/frontend/src/shared/i18n/locales/`
Applied to files:
apps/frontend/src/renderer/components/task-detail/SubtaskActionList.tsxapps/frontend/src/renderer/lib/actionScoring.ts
📚 Learning: 2025-12-25T18:29:32.954Z
Learnt from: CR
Repo: AndyMik90/Auto-Claude PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-12-25T18:29:32.954Z
Learning: Applies to apps/frontend/src/**/*.{ts,tsx} : Always use translation keys with `useTranslation()` for all user-facing text in React/TypeScript frontend components - use format `namespace:section.key` (e.g., `navigation:items.githubPRs`)
Applied to files:
apps/frontend/src/renderer/components/task-detail/SubtaskActionList.tsxapps/frontend/src/renderer/lib/actionScoring.ts
🧬 Code graph analysis (3)
apps/frontend/src/renderer/components/task-detail/SubtaskActionList.tsx (4)
apps/frontend/src/renderer/lib/actionScoring.ts (3)
ScoredAction(36-41)getScoreReasonKeys(405-436)groupActionsBySubphase(384-397).design-system/src/lib/utils.ts (1)
cn(4-6).design-system/src/components/Badge.tsx (1)
Badge(9-27)apps/frontend/scripts/download-python.cjs (1)
key(106-106)
apps/frontend/src/renderer/components/task-detail/TaskSubtasks.tsx (2)
apps/frontend/src/shared/types/task.ts (3)
Subtask(32-43)Task(235-255)TaskLogEntry(64-77)apps/frontend/src/renderer/lib/actionScoring.ts (4)
filterTopActions(341-376)calculateSubtaskRelevanceScores(557-568)sortSubtasksByRelevance(581-615)getImportantFiles(864-879)
apps/frontend/src/renderer/lib/actionScoring.ts (1)
apps/frontend/src/shared/types/task.ts (2)
TaskLogEntry(64-77)TaskLogEntryType(62-62)
🔇 Additional comments (22)
apps/frontend/src/shared/i18n/locales/en/tasks.json (1)
86-126: LGTM! Well-structured i18n keys for the new subtask action summarization feature.The new
detailnamespace with nested sections (subtasks,actions,tools,scoring) follows a logical structure that aligns with the UI components. The interpolation patterns (e.g.,{{completed}},{{displayed}}) are correctly formatted for i18next.apps/frontend/src/shared/i18n/locales/fr/tasks.json (1)
86-126: LGTM! French translations are complete and maintain parity with the English file.All translation keys added in the English file are properly mirrored here with French translations. The interpolation placeholders (
{{completed}},{{total}},{{displayed}}) are correctly preserved. Based on learnings, this satisfies the requirement to add new translation keys to ALL language files.apps/frontend/src/renderer/lib/__tests__/actionScoring.test.ts (1)
1-39: LGTM! Comprehensive test suite with excellent coverage.The test file thoroughly covers:
- All exported constants and interfaces
- Predicate functions (isErrorAction, isWarningAction, isDecisionAction, isFileChangeAction, isNovelty)
- Core scoring logic with score breakdown validation
- Edge cases (null/undefined, empty, single action, N=0, negative N)
- Performance benchmarks with CI-aware thresholds
- Integration scenarios with realistic action mixes
- Subtask relevance scoring and sorting
- File extraction and summarization
The helper functions
createTestActionandcreateScoringContextprovide clean test setup.apps/frontend/src/renderer/lib/actionScoring.ts (5)
1-52: LGTM! Well-documented module with clear cognitive science rationale.The module header clearly documents the scoring weights and their research-backed justifications. The exported interfaces (
ScoredAction,ScoreBreakdown,ScoringContext) provide good type safety for consumers.
195-205: Good documentation of the unimplemented feature.The TODO clearly explains why
parseDurationreturns undefined and what's needed to enable TIME_ANOMALY scoring. This prevents confusion about the dead code path.
282-288: Potential unbounded score from file change multiplier.The
fileChangescore is multiplied byMath.max(1, filesChanged), butfilesChangedis capped at 3 bycountFilesChanged. However, the comment says "cap at 3x base weight" which is correct behavior. This is fine as-is.
405-436: Clean i18n-ready implementation.
getScoreReasonKeyscorrectly returns translation keys that match the keys defined intasks.json. The deprecatedgetScoreReasonis preserved for backwards compatibility with appropriate@deprecatedJSDoc.
581-615: Efficient sorting with pre-computed index map.Good use of the index map for O(1) lookups in the tertiary sort, addressing the previous O(n²) concern. The composite score formula clearly prioritizes errors (1000 bonus) over other factors.
apps/frontend/src/renderer/components/task-detail/TaskDetailModal.tsx (3)
1-1: LGTM! Appropriate import addition.The
useCallbackimport is correctly added to support the memoized callback.
159-162: LGTM! Well-memoized callback.The
handleViewAllLogscallback is correctly memoized withuseCallbackand the appropriate dependency (state.setActiveTab). This prevents unnecessary re-renders ofTaskSubtaskswhen other state changes.
436-440: LGTM! Proper prop wiring to TaskSubtasks.The
phaseLogsandonViewAllLogsprops are correctly passed, enabling the subtasks component to access phase log data and navigate to the logs tab.apps/frontend/src/renderer/components/task-detail/SubtaskActionList.tsx (6)
1-43: LGTM! Clean imports and well-typed interface.The component imports are appropriate, and
SubtaskActionListPropsis well-documented with JSDoc comments explaining each prop's purpose.
49-67: Good i18n pattern with fallback for unknown tools.The
getToolInfofunction correctly returns translation keys for known tools and provides a fallback label for unknown tools. The pattern of returninglabelKey: nullwith afallbackLabelis handled properly in the consumer.
105-177: LGTM! Well-structured tool action rendering.The
ScoredActionItemcomponent correctly handles both tool actions and typed entries with proper i18n usage. Thetype="button"attributes are present on interactive elements.
241-271: LGTM! Clean ScoreBadge implementation.The component correctly uses
getScoreReasonKeyswith thePicktype (no unsafe assertion needed sincegetScoreReasonKeysacceptsPick<ScoredAction, 'scoreBreakdown'>). Translation keys are properly resolved viat().
282-321: LGTM! SubphaseGroup with proper accessibility.The collapsible group uses
type="button"on the trigger and provides clear visual feedback for expansion state.
420-503: LGTM! Main component with proper memoization and i18n.The
SubtaskActionListcomponent:
- Uses
useMemoappropriately fordisplayActionsandgroupedActions- All user-facing strings use translation keys
- Handles empty state gracefully
- Uses
type="button"on the "View all logs" buttonapps/frontend/src/renderer/components/task-detail/TaskSubtasks.tsx (5)
1-11: LGTM! Clean imports with no unused dependencies.The imports are appropriate for the component's functionality. The
ArrowUpDownicon was correctly removed as noted in past reviews.
40-65: LGTM! Helper functions with clear responsibilities.
getAllEntriesFromPhaseLogsefficiently flattens phase log entries, andgetSubtaskActionCountprovides a simple count API. Both handle null/undefined inputs gracefully.
95-126: LGTM! Well-structured memoization strategy.
allEntriesis memoized based onphaseLogsgetSubtaskActionsis auseCallbackthat capturesallEntriesrelevanceScoresrecalculates when entries or subtasks changesortedSubtasksuses a proper type guard:.filter((s): s is Subtask => s != null)This addresses the previous concern about non-null assertions.
102-105: getSubtaskActions recreates on every allEntries change but is called conditionally.The
useCallbackforgetSubtaskActionsis recreated wheneverallEntrieschanges. Since it's only called whenisExpandedis true, this is fine. However, consider if this could cause issues if many subtasks are expanded simultaneously.This is informational - the current implementation is correct. Each expanded subtask will call
filterTopActionswhich scores and sorts, potentially multiple times ifallEntrieschanges frequently. For the expected use case (5-10 subtasks, few expanded at once), this should be acceptable.
328-342: LGTM! Clean integration with SubtaskActionList.The
SubtaskActionListis rendered with appropriate props, including the memoizedscoredActionsand file information. The conditional rendering based onhasActionsensures the component only appears when relevant.
apps/frontend/src/renderer/components/task-detail/TaskSubtasks.tsx
Outdated
Show resolved
Hide resolved
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
kvnloo
changed the title
Implement cognitively-optimized expandable view for subtasks that intelligently surfaces the 5 most relevant agent actions from logs containing 1000+ actions. Add comprehensive action scoring algorithm with tests to create a middle layer between raw logs and user interface for better information clarity. Signed-off-by: kevin rajan <[email protected]>
Fixes AndyMik90#235 The Edit Task modal's close button (X) was unresponsive because both the parent modal and the edit dialog used z-50 for their overlays. The parent's overlay intercepted clicks meant for the edit dialog's close button. This fix hides the parent modal while the Edit dialog is open, then reopens it when the Edit dialog closes. This is a cleaner UX than z-index hacks. Signed-off-by: Black Circle Sentinel <[email protected]> Signed-off-by: kevin rajan <[email protected]>
* fix: make backend tests pass on Windows * fix: address Windows locking + lazy graphiti imports * fix: address CodeRabbit review comments * fix: improve file_lock typing * Update apps/backend/runners/github/file_lock.py Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> * fix: satisfy ruff + asyncio loop usage * style: ruff format file_lock * refactor: safer temp file close + async JSON read * style: ruff format file_lock --------- Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Andy <[email protected]> Signed-off-by: kevin rajan <[email protected]>
AndyMik90#351) * fix(analyzer): add C#/Java/Swift/Kotlin project files to security hash Fixes AndyMik90#222 The security profile hash calculation was missing key config files for several languages, causing the profile not to regenerate when: - C# projects (.csproj, .sln, .fsproj, .vbproj) changed - Java/Kotlin/Scala projects (pom.xml, build.gradle, etc.) changed - Swift packages (Package.swift) changed Changes: - Add Java, Kotlin, Scala, Swift config files to hash_files list - Add glob patterns for .NET project files (can be anywhere in tree) - Update fallback source extensions to include .cs, .swift, .kt, .java Signed-off-by: Black Circle Sentinel <[email protected]> * fix(analyzer): replace empty except pass with continue --------- Signed-off-by: Black Circle Sentinel <[email protected]> Co-authored-by: Andy <[email protected]> Signed-off-by: kevin rajan <[email protected]>
…ik90#358) * fix(terminal): preserve terminal state when switching projects Fixes terminal state loss when switching between project tabs (AndyMik90#342). Two issues addressed: 1. PTY health check: Added checkTerminalPtyAlive IPC method to detect terminals with stale state (no live PTY process). restoreTerminalSessions now removes dead terminals and restores from disk instead of skipping. 2. Buffer preservation: Added SerializeAddon to capture terminal buffer with ANSI escape codes before disposal. This preserves the shell prompt, colors, and output history when switching back to a project. Closes AndyMik90#342 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]> * fix(terminal): address PR review findings Addresses 5 findings from Auto Claude PR Review: 1. [HIGH] Race condition protection: Added restoringProjects Set to prevent concurrent restore calls for the same project 2. [HIGH] Unnecessary disk restore: Skip disk restore when some terminals are still alive to avoid duplicates 3. [HIGH] Double dispose vulnerability: Added isDisposedRef guard to prevent corrupted serialization on rapid unmount/StrictMode 4. [MEDIUM] SerializeAddon disposal: Explicitly call dispose() before setting ref to null 5. [MEDIUM] projectPath validation: Added input validation at function start 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]> * fix(terminal): allow disk restore when alive terminals exist CodeRabbit review finding: When a project has mixed alive and dead terminals, the early return was preventing disk restore, causing dead terminals to be permanently lost. The fix removes the early return since addRestoredTerminal() already has duplicate protection (checks terminal ID before adding). This allows dead terminals to be safely restored from disk while alive terminals remain unaffected. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]> * fix(terminal): remove unused aliveTerminals variable CodeQL flagged unused variable after previous fix removed the early return that was using it. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]> --------- Co-authored-by: Claude Opus 4.5 <[email protected]> Signed-off-by: kevin rajan <[email protected]>
- Add getScoreReasonKeys() to return translation keys instead of hardcoded strings - Convert tool labels in SubtaskActionList to use i18n translation keys - Add comprehensive translations for score reasons in en/fr tasks.json - Consolidate filterTopActions logic and improve edge case handling - Update tests to verify i18n key generation 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]> Signed-off-by: kevin rajan <[email protected]>
… ruff version, and broken quality-dco workflow (AndyMik90#334) * fix: Fixes issues to get clean pre-commit run 1. version-sync hook was failing due to formatting, fixed with block scalar. 2. Python Tests step was failing because it could not locate python or pytest. Fixed by referencing pytest in .venv, [as was shown here in CONTRIBUTING.md](https://github.com/AndyMik90/Auto-Claude/blob/develop/CONTRIBUTING.md?plain=1#L299) At this point pre-commit could run, then there were a few issues it found that had to be fixed: 3. "check yaml" hook failed for the file ".github/workflows/quality-dco.yml". Fixed indenting issue. 4. Various files had whitespace issues that were auto-fixed by the pre-commit commands. After this, "pre-commit run --all-files" passes for all checks. Signed-off-by: Ian <[email protected]> * docs: Update CONTRIBUTING.md with cmake dependency cmake is not present by default on macs, can be installed via homebrew Signed-off-by: Ian <[email protected]> * Addressed PR comments on file consistency and install instructions. Signed-off-by: Ian <[email protected]> * Ran pre-commit autoupdate, disabled broken quality-dco workflow The version of ruff in pre-commit was on a much older version than what was running as part of the lint github workflow. This caused it to make changes that were rejected by the newer version. As far as disabling quality-dco workflow; according to https://github.com/AndyMik90/Auto-Claude/actions/workflows/quality-dco.yml, it has never actually successfully parsed since it was introduced in AndyMik90#266, and so it has not been running on any PRs to date. Given that, plus the fact that I see no mention/discussion of Developer Certificate of Origin in any github issues or the discord, I will run with the assumption this needs more explicit discussion before we turn it on and force all contributors to add these signoffs for every commit. Signed-off-by: Ian <[email protected]> * Fixed bad sed command in pre-commit version-sync hook It resulted in bad version names being produced for beta versions, such as "Auto-Claude-2.7.2-beta.9-beta.9-arm64.dmg". Also addressed PR comment for needed spacing in markdown code blocks. Signed-off-by: Ian <[email protected]> * Fixed other sed command for version sync to avoid incorrect names Addresses PR comment to keep this in line with existing sed command fix in the same PR. Signed-off-by: Ian <[email protected]> * Keep version of ruff in sync between pre-commit and github workflow This will avoid situations where the checks done locally and in CI start to diverge and even conflict Signed-off-by: Ian <[email protected]> * Enabling DCO workflow Signed-off-by: Ian <[email protected]> * Fixed windows compatibility issue for running pytest Also committing some more file whitespace changes made by the working pre-commit hook. Signed-off-by: Ian <[email protected]> * Removed out of date disabled banner on quality dco workflow Signed-off-by: Ian <[email protected]> * Fixed version-sync issue with incorrect version badge image url, fixed dco workflow Updated readme with correct value as well Fixed DCO workflow as it was pointing at a nonexistent step. Improved DCO workflow failure message to warn about accidentally signing others commits. --------- Signed-off-by: Ian <[email protected]> Co-authored-by: Andy <[email protected]> Signed-off-by: kevin rajan <[email protected]>
* fix(subprocess): handle Python paths with spaces Fixes AndyMik90#315 The packaged macOS app uses a Python path inside ~/Library/Application Support/ which contains a space. The subprocess-runner.ts was passing the path directly to spawn(), causing ENOENT errors. This fix adds parsePythonCommand() (already used by agent-process.ts) to properly handle paths with spaces. This also affects Changelog generation and other GitHub automation features. Signed-off-by: Black Circle Sentinel <[email protected]> * test(subprocess): add unit tests for python path spaces and arg ordering --------- Signed-off-by: Black Circle Sentinel <[email protected]> Co-authored-by: Andy <[email protected]> Signed-off-by: kevin rajan <[email protected]>
…AndyMik90#355) * fix(security): invalidate profile cache when file is created/modified Fixes AndyMik90#153 The security profile cache was returning stale data even after the .auto-claude-security.json file was created or updated. This caused commands like 'dotnet' to be blocked even when present in the file. Root cause: get_security_profile() cached the profile on first call without checking if the file's mtime changed on subsequent calls. Fix: Track the security profile file's mtime and invalidate the cache when the file is created (mtime goes from None to a value) or modified (mtime changes). This also helps with issue AndyMik90#222 where the profile is created after the agent starts - now the agent will pick up the new profile on the next command validation. Signed-off-by: Black Circle Sentinel <[email protected]> * fix(security): handle deleted profile file and add cache invalidation tests * fix(security): handle deleted profile file and add cache invalidation tests * test(security): improve cache tests with mocks and unique commands * test(security): add mock-free tests for cache invalidation * test(security): fix cache invalidation tests without mocks * fix(security): address review comments and add debug logs for CI hash failure * fix(analyzer): remove debug prints * fix(lint): sort imports in profile.py * fix(security): include spec_dir in cache key to prevent stale profiles The cache key previously only included project_dir, but the profile location can depend on spec_dir. This could cause stale cached profiles to be returned if spec_dir changes between calls. Fix: Add _cached_spec_dir to the cache validation logic and reset function. --------- Signed-off-by: Black Circle Sentinel <[email protected]> Co-authored-by: Andy <[email protected]> Signed-off-by: kevin rajan <[email protected]>
…op (AndyMik90#362) The projectTabs array was being included in the useEffect dependency array, but since it's computed fresh on every render (via getProjectTabs()), it always has a new reference. This caused the effect to fire on every render cycle, creating an infinite re-render loop. Fix: Use openProjectIds.includes() instead of projectTabs.some() since openProjectIds is stable state and already tracks the same information. Fixes performance regression in beta.10 where UI interactions took 5-6 seconds. Co-authored-by: Andy <[email protected]> Signed-off-by: kevin rajan <[email protected]>
- Fix formatTime to respect user locale (use undefined instead of 'en-US') - Fix getFilenameFromPath to handle Windows paths (split on /[\\/]/) - Fix O(n²) findIndex in TaskSubtasks by precomputing index map - Add i18n translations for tooltip strings (actionsCount, averageScore, topScore) - Fix tiebreaker test to assert exact original order for ties - Fix quoted-path regex: move quoted patterns first, remove /g flags, use exec() - Fix CodeQL warning: combine undefined checks in isTimeAnomaly 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <[email protected]> Signed-off-by: kevin rajan <[email protected]>
kvnloo
force-pushed
the
feat/enhanceSubtaskView
branch
from
cc49c2a to
daa88db
Compare
|
|
7 participants
Base Branch
developbranch (required for all feature/fix PRs)main(hotfix only - maintainers)Description
Implement a cognitively-optimized expandable view for subtasks that intelligently surfaces the 5 most relevant agent actions from logs containing 1000+ actions. This creates a critical "middle layer" between high-level subtask summaries and overwhelming full logs, using weighted scoring algorithms based on cognitive science principles (Justin Sung, Scott Young methodologies) and progressive disclosure UI patterns to maintain developer flow state during debugging and monitoring.
Related Issue
Closes #
Type of Change
Area
Commit Message Format
Follow conventional commits:
feat: add intelligent action summarization UITypes: feat, fix, docs, style, refactor, test, chore
Example:
feat: add user authentication systemChecklist
developbranchCI/Testing Requirements
Screenshots
Feature Toggle
use_feature_nameBreaking Changes
Breaking: No
Details:
Summary by CodeRabbit
New Features
Tests
✏️ Tip: You can customize this high-level summary in your review settings.