Skip to content

Conversation

axin7
Copy link
Contributor

@axin7 axin7 commented Sep 27, 2025

Summary

Added intelligent session creation functionality with keyboard shortcut that automatically detects the current project context and creates a new chat session.

Features

  • Keyboard shortcut: cmd+n (Mac) / ctrl+n (other platforms)
  • Smart project detection: Automatically detects project from active tab or recent chat tabs
  • Automatic project assignment: Extracts and assigns project name to new session
  • Graceful fallback: Creates generic session when no project context is found

Technical Changes

  • Added getCurrentProjectPath() in useTabState hook to detect current project context
  • Added createNewSessionWithCurrentProject() helper function for intelligent session creation
  • Registered 'create-new-session' custom event handler in App.tsx
  • Updated TypeScript types for new functions

@123vivekr
Copy link
Member

@claude review this

Copy link

claude bot commented Oct 10, 2025

Claude finished @123vivekr's task —— View job


Claude Code is reviewing the PR...

Review Tasks:

  • Review the Rust backend changes in commands/claude.rs
  • Review the TypeScript implementation in App.tsx
  • Review the FloatingPromptInput.tsx changes
  • Review the TabContent.tsx changes
  • Review the TabManager.tsx changes
  • Review the useTabState.ts hook implementation
  • Check Cargo.toml and Cargo.lock changes
  • Provide comprehensive review feedback

Code Review

Overall Assessment

This PR implements an intelligent session creation feature with cmd+n keyboard shortcut. The implementation is mostly solid but has some areas that need attention.

Strengths

  • Clean separation of concerns between project detection logic and session creation
  • Good integration with existing tab management system
  • Appropriate use of TypeScript types and React hooks
  • Graceful fallback behavior when no project context is found

Issues and Suggestions

1. Backend Implementation (commands/claude.rs)

The backend changes look minimal and focused on dependency updates. No issues found.

2. App.tsx Implementation

⚠️ Missing keyboard shortcut handler: The PR description mentions adding cmd+n shortcut, but I don't see it implemented in App.tsx. The keyboard handlers only include:

  • cmd+t: create chat tab
  • cmd+w: close tab
  • cmd+tab: switch tabs
  • cmd+1-9: switch to tab by index

Suggested fix - Add this to the keyboard handler in App.tsx:

case 'n':
  if (!e.shiftKey) { // Ensure it's not cmd+shift+n
    e.preventDefault();
    window.dispatchEvent(new CustomEvent('create-new-session'));
  }
  break;
3. useTabState.ts Implementation

✅ Good implementation of getCurrentProjectPath() - but it's missing from the return object of the hook. The function is not exposed for other components to use.

Issue: The PR claims to add getCurrentProjectPath() but I don't see it in the hook's implementation or exports.

Suggested fix - Add to useTabState.ts:

const getCurrentProjectPath = useCallback((): string | undefined => {
  if (!activeTab) return undefined;
  
  // For chat tabs, use initialProjectPath or sessionData.project_path
  if (activeTab.type === 'chat') {
    return activeTab.initialProjectPath || activeTab.sessionData?.project_path;
  }
  
  // For other tabs that might have project context
  if ('projectPath' in activeTab) {
    return activeTab.projectPath;
  }
  
  return undefined;
}, [activeTab]);

// Add to return object:
return {
  // ... existing returns
  getCurrentProjectPath,
};
4. Missing createNewSessionWithCurrentProject Implementation

The PR description mentions this function but I don't see it implemented anywhere.

Suggested implementation:

const createNewSessionWithCurrentProject = useCallback((): string => {
  const currentProjectPath = getCurrentProjectPath();
  const projectName = currentProjectPath?.split('/').pop() || 'New Session';
  
  return createChatTab(
    undefined, // No session ID for new session
    projectName,
    currentProjectPath
  );
}, [getCurrentProjectPath, createChatTab]);
5. TabContent.tsx & TabManager.tsx

✅ The tab management components look good and properly handle project context passing.

6. FloatingPromptInput.tsx

⚠️ The changes seem unrelated to the PR's stated purpose - they appear to be focused on image handling improvements rather than session creation.

Security Considerations

  • ✅ No security issues identified
  • ✅ Proper path handling without exposing sensitive information
  • ✅ No hardcoded credentials or secrets

Performance Considerations

  • The project detection logic is lightweight and shouldn't impact performance
  • Tab creation is already optimized in the existing implementation

Missing Tests

The PR lacks tests for the new functionality. Consider adding tests for:

  • Project path detection from active tabs
  • Fallback behavior when no project is found
  • Keyboard shortcut handling

Recommendations

  1. Complete the implementation: The keyboard shortcut handler and helper functions mentioned in the PR description are missing.

  2. Add the keyboard shortcut: Implement the cmd+n handler in App.tsx to trigger the new session creation.

  3. Expose getCurrentProjectPath: Make sure this function is properly implemented and exported from useTabState.

  4. Remove unrelated changes: The FloatingPromptInput.tsx changes don't seem related to this PR's purpose.

  5. Add documentation: Update the keyboard shortcuts documentation to include cmd+n.

  6. Consider edge cases:

    • What happens when multiple tabs are open from different projects?
    • Should it prefer the most recently active chat tab's project?

The core concept is good, but the implementation appears incomplete based on the PR description. Please address the missing components before merging.

@123vivekr
Copy link
Member

hi @axin7 can you take a look at the suggestions and also fix the merge conflicts?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants