feat(tree): add multi-root directory labels#1024
Conversation
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. 📝 WalkthroughWalkthroughThis PR introduces multi-root repository support by adding file grouping and root-aware tree generation. It threads an optional Changes
Sequence Diagram(s)sequenceDiagram
participant Packager as src/core/packager.ts
participant ProduceOutput as src/core/packager/produceOutput.ts
participant OutputSplit as src/core/output/outputSplit.ts
participant OutputGen as src/core/output/outputGenerate.ts
participant TreeGen as src/core/file/fileTreeGenerate.ts
Packager->>Packager: Build FilesByRoot from sortedFilePathsByDir
Packager->>ProduceOutput: produceOutput(filePathsByRoot)
ProduceOutput->>OutputSplit: generateSplitOutputParts(filePathsByRoot)
OutputSplit->>OutputSplit: renderGroups(filePathsByRoot)
OutputSplit->>OutputGen: generateOutput(filePathsByRoot)
alt Multiple roots provided
OutputGen->>TreeGen: generateTreeStringWithRoots(filePathsByRoot)
TreeGen->>TreeGen: Group files by root, skip empty
TreeGen-->>OutputGen: Root-labeled tree string
else Single root or undefined
OutputGen->>TreeGen: generateTreeString(filePathsForTree)
TreeGen-->>OutputGen: Flat tree string
end
OutputGen-->>OutputSplit: Output with labeled tree
OutputSplit-->>ProduceOutput: Split output parts
ProduceOutput-->>Packager: Final output generated
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (5 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Summary of ChangesHello @spandan-kumar, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly improves the clarity of the directory tree output when the system processes files from multiple distinct root directories. By introducing automatic labeling for each root, users can now easily discern which files belong to which source directory, enhancing the overall readability and organization of the generated output. This change is particularly beneficial for projects with complex, multi-component structures. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a great feature for labeling directory trees when multiple root directories are provided. The implementation is solid and includes good test coverage. I've identified a couple of opportunities to refactor the code for better maintainability by reducing code duplication and simplifying logic. My suggestions focus on consolidating the new tree generation functions in fileTreeGenerate.ts and simplifying the conditional logic in outputGenerate.ts.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
tests/core/file/fileTreeGenerate.test.ts (1)
2-8: Unused imports.
generateFileTreeandtreeToStringare imported but not used in the tests. Consider removing them to keep imports clean, or add tests that exercise these functions directly.🔎 Proposed fix
import { - type FilesByRoot, - generateFileTree, generateTreeString, generateTreeStringWithRoots, - treeToString, + type FilesByRoot, } from '../../../src/core/file/fileTreeGenerate.js';src/core/output/outputGenerate.ts (1)
377-385: Simplify the condition to improve consistency.The current logic checks
filePathsByRoot.length > 1, which means when exactly one root is provided, it falls back to usingfilePathsForTreeinstead offilePathsByRoot[0].files. SincegenerateTreeStringWithRootsalready handles the single-root case internally, this creates an inconsistency where single-root and multi-root scenarios use different data sources.Consider simplifying to check only for the presence of
filePathsByRoot, lettinggenerateTreeStringWithRootshandle both single and multiple roots uniformly.🔎 Proposed simplification
// Generate tree string - use multi-root format if multiple roots are provided let treeString: string; -if (filePathsByRoot && filePathsByRoot.length > 1) { +if (filePathsByRoot) { // Multiple roots: generate labeled tree sections treeString = generateTreeStringWithRoots(filePathsByRoot, directoryPathsForTree); } else { // Single root or no root info: use standard flat tree treeString = generateTreeString(filePathsForTree, directoryPathsForTree); }
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
src/core/file/fileTreeGenerate.tssrc/core/output/outputGenerate.tssrc/core/output/outputSplit.tssrc/core/packager.tssrc/core/packager/produceOutput.tstests/core/file/fileTreeGenerate.test.tstests/core/output/flagFullDirectoryStructure.test.tstests/core/output/outputGenerate.test.tstests/core/output/outputGenerateDiffs.test.ts
🧰 Additional context used
🧬 Code graph analysis (4)
src/core/packager/produceOutput.ts (2)
src/core/file/fileTreeGenerate.ts (1)
FilesByRoot(120-123)src/shared/memoryUtils.ts (1)
withMemoryLogging(71-87)
src/core/output/outputSplit.ts (2)
src/core/file/fileTreeGenerate.ts (1)
FilesByRoot(120-123)src/shared/errorHandle.ts (1)
RepomixError(6-11)
src/core/packager.ts (1)
src/core/file/fileTreeGenerate.ts (1)
FilesByRoot(120-123)
src/core/output/outputGenerate.ts (2)
src/core/file/fileTreeGenerate.ts (3)
FilesByRoot(120-123)generateTreeStringWithRoots(133-153)generateTreeString(103-106)src/index.ts (1)
generateTreeString(13-13)
🔇 Additional comments (18)
tests/core/output/outputGenerate.test.ts (1)
44-65: LGTM!The test correctly threads the new
undefinedargument forfilePathsByRootthrough bothgenerateOutputandbuildOutputGeneratorContextcalls when using mock dependencies. The assertions remain valid and properly verify the expected behavior.tests/core/output/flagFullDirectoryStructure.test.ts (2)
67-76: LGTM!The additional
undefinedargument forfilePathsByRootis correctly positioned before thedepsparameter, maintaining proper alignment with the updatedbuildOutputGeneratorContextsignature.
100-109: LGTM!Consistent with the first test case - the
undefinedargument is correctly threaded through.tests/core/output/outputGenerateDiffs.test.ts (1)
83-92: LGTM!The additional
undefinedargument forfilePathsByRootis correctly added beforemockDepsacross all test cases in this file. The pattern is consistent and aligns with the updatedgenerateOutputsignature.src/core/packager.ts (1)
1-9: LGTM!The new imports are correctly added for
node:pathandFilesByRoottype, supporting the multi-root tree generation feature.tests/core/file/fileTreeGenerate.test.ts (1)
1-96: Good test coverage for multi-root tree generation.The tests comprehensively cover the key behaviors:
- Single root fallback (no labels)
- Multiple roots with labeled sections
- Nested directory handling
- Empty root section skipping
- Parity verification between single-root and standard generation
src/core/packager/produceOutput.ts (3)
24-65: LGTM!The
filePathsByRootparameter is cleanly threaded through the publicproduceOutputfunction and correctly passed to bothgenerateAndWriteSplitOutputandgenerateAndWriteSingleOutputinternal helpers. The optional parameter positioning beforeoverrideDepsis consistent and appropriate.
67-94: LGTM!The
generateAndWriteSplitOutputfunction correctly receives and forwardsfilePathsByRoottogenerateSplitOutputParts.
118-131: LGTM!The
generateAndWriteSingleOutputfunction correctly passesfilePathsByRoottodeps.generateOutput.src/core/file/fileTreeGenerate.ts (2)
117-123: LGTM!The
FilesByRootinterface is clean and well-documented, providing a simple structure for grouping files by their root directory label.
164-188: LGTM with same caveat.The
generateTreeStringWithRootsAndLineCountsfunction follows the same pattern asgenerateTreeStringWithRoots, which is good for consistency. The same consideration aboutemptyDirPathshandling applies here.src/core/output/outputSplit.ts (4)
96-119: LGTM!The
renderGroupsfunction correctly accepts and forwardsfilePathsByRootto thegenerateOutputcall. The parameter is appropriately placed and the threading is consistent.
121-145: LGTM!The
generateSplitOutputPartsfunction signature is correctly updated to include the optionalfilePathsByRootparameter in both the function parameters and the destructured type definition.
169-182: LGTM!The first
renderGroupscall in the group iteration loop correctly passesfilePathsByRoot.
210-219: LGTM!The second
renderGroupscall (for single group content) also correctly passesfilePathsByRoot, maintaining consistency in the split output path.src/core/output/outputGenerate.ts (3)
8-8: LGTM!The imports correctly bring in the new
FilesByRoottype andgenerateTreeStringWithRootsfunction needed for multi-root support, while preserving the existinggenerateTreeStringimport for backward compatibility.
253-296: LGTM!The optional
filePathsByRootparameter is properly threaded through tobuildOutputGeneratorContext, maintaining backward compatibility while enabling multi-root support.
298-311: LGTM!The optional
filePathsByRootparameter is correctly added to the function signature with proper typing.
When packing multiple directories, the directory tree output now shows labeled sections like [cli]/, [config]/ to clarify which files belong to which root directory. - Add FilesByRoot interface and generateTreeStringWithRoots function - Update output pipeline to pass file-to-root mapping - Add unit tests for new tree generation functions - Update existing tests for new function signatures Closes yamadashy#1023
yamadashy
force-pushed
the
feat/multi-root-directory-tree-labels-1023
branch
from
614985b to
3f2680e
Compare
…Root - Remove unused imports (generateFileTree, treeToString) in fileTreeGenerate.test.ts - Add filePathsByRoot parameter to generateOutput and produceOutput calls in tests - Update expect assertions to include filePathsByRoot argument
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1024 +/- ##
==========================================
- Coverage 87.19% 87.15% -0.04%
==========================================
Files 116 116
Lines 4350 4376 +26
Branches 1011 1019 +8
==========================================
+ Hits 3793 3814 +21
- Misses 557 562 +5 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
- Extract common multi-root logic into generateMultiRootSections helper - Simplify conditional in outputGenerate.ts (generateTreeStringWithRoots handles single root internally) - Add fallback for empty path.basename (e.g., filesystem root "/") - Document limitation: empty directories not included in multi-root output
|
Hi @spandan-kumar! Thank you for the suggestion and this PR! This is a great idea as a feature, and the implementation looks good. |
|
@spandan-kumar |
Add release notes for v1.11.1 covering: - Multi-root directory labels (#1024) - Non-interactive skill generation options (#1022) - Remote git command timeout fix (#1078) - CLI output visibility fix for light themes (#1088) - Library bundling documentation (#1075) Improve release note generation guidelines: - Clarify "What's New" vs "Improvements" usage - Add rule to include related issue numbers with PRs - Add rule to include links in documentation sections
2 participants
When packing multiple directories, the directory tree output now shows labeled sections like [cli]/, [config]/ to clarify which files belong to which root directory.
Closes #1023
Files Changed
filePathsByRootthroughtests/core/output/*.test.ts- Updated 3 test files for new signaturesSummary
Added automatic root directory labels to the directory tree output when packing multiple directories. This provides clear visual grouping showing which files belong to which root directory.