feat: add dotnet recommendation support#135
Open
FrozenPhoenix92 wants to merge 5 commits into
Open
Conversation
|
@FrozenPhoenix92 is attempting to deploy a commit to the rohitg00's projects Team on Vercel. A member of the Team first needs to authorize it. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughWalkthroughAdds .NET project detection to Changes.NET ProjectDetector Extension
Tap-Based Recommendation Sources
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (2)
packages/cli/src/recommend-sources.ts (1)
5-9: ⚡ Quick winUse the shared core
SkillRepoSourcetype instead of redefining it.Line 5 duplicates the contract already exported from core; importing the shared type avoids drift between CLI and core over time.
Suggested diff
import type { TapEntry } from './helpers.js'; +import type { SkillRepoSource } from '`@skillkit/core`'; const GITHUB_REPO_PATTERN = /^([\w.-]+)\/([\w.-]+)$/; -export interface SkillRepoSource { - owner: string; - repo: string; - description?: string; -} - export interface RecommendationSourcesResult { sources: SkillRepoSource[]; warnings: string[]; }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/cli/src/recommend-sources.ts` around lines 5 - 9, The SkillRepoSource interface defined in packages/cli/src/recommend-sources.ts is duplicating an existing type from the shared core package. Remove the local interface definition (lines 5-9) and instead import SkillRepoSource from the core package to maintain a single source of truth and prevent type drift between the CLI and core implementations.packages/cli/src/__tests__/recommend-sources.test.ts (1)
35-48: ⚡ Quick winAdd a mixed-case dedupe assertion for owner/repo.
This test currently validates exact-case duplicates only; add one
Some-Org/Some-Skillsvariant to lock in the case-insensitive dedupe contract.Suggested diff
it('should dedupe duplicate owner/repo sources', () => { const builtIn = knownSkillRepos[0]; const taps: TapEntry[] = [ { source: `${builtIn.owner}/${builtIn.repo}`, addedAt: '2026-06-14T00:00:00.000Z' }, { source: 'some-org/some-skills', addedAt: '2026-06-14T00:00:00.000Z' }, + { source: 'Some-Org/Some-Skills', addedAt: '2026-06-14T00:00:00.000Z' }, { source: 'some-org/some-skills', addedAt: '2026-06-14T00:00:00.000Z' }, ];🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/cli/src/__tests__/recommend-sources.test.ts` around lines 35 - 48, The test in the should dedupe duplicate owner/repo sources block currently only validates exact-case duplicate deduplication. Add a mixed-case variant of an existing source (such as Some-Org/Some-Skills alongside some-org/some-skills) to the taps array to test case-insensitive deduplication, and then add an assertion using the same pattern as the existing expects to verify that the mixed-case variant is also deduplicated to a length of 1, ensuring the deduplication logic handles case-insensitive owner/repo matching.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/cli/src/commands/recommend.ts`:
- Line 693: The console.log() call at line 693 that prints the resolved sources
always executes and does not respect the --quiet or --json command-line flags,
causing unwanted output to leak in those modes. Add a conditional check before
this console.log() statement to only print the sources when neither quiet nor
json mode is enabled. Determine how quiet and json flags are tracked in this
command (they may be part of the options/config object passed to the command)
and wrap the sources output with a condition that skips printing when either
flag is active.
In `@packages/core/src/context/detector.ts`:
- Around line 823-830: The current implementation of hasFile('global.json') only
checks for the file at the repository root, missing nested global.json files
commonly found in monorepos. Modify the logic in the getDotNetVersion method
(around lines 823-830 and the referenced lines 955-961) to search for
global.json across scanned paths and subdirectories, not just the root level.
Update the source determination in the return statement to reflect whether the
detected version comes from a root-level or nested global.json file by changing
how hasFile('global.json') is invoked or by creating a method that searches
through all scanned path directories for the presence of global.json.
- Around line 808-816: The issue is in the C# language detection logic where the
condition uses OR to check both for `.csproj` files and solution files. This
incorrectly classifies F#-only or VB-only solutions as C#. Remove the call to
this.hasDotNetSolution() from the condition so that C# is only inferred when
`.csproj` files are actually present. Update the source assignment to use only
the result from this.firstDotNetFile('.csproj') since the solution file check is
being removed.
---
Nitpick comments:
In `@packages/cli/src/__tests__/recommend-sources.test.ts`:
- Around line 35-48: The test in the should dedupe duplicate owner/repo sources
block currently only validates exact-case duplicate deduplication. Add a
mixed-case variant of an existing source (such as Some-Org/Some-Skills alongside
some-org/some-skills) to the taps array to test case-insensitive deduplication,
and then add an assertion using the same pattern as the existing expects to
verify that the mixed-case variant is also deduplicated to a length of 1,
ensuring the deduplication logic handles case-insensitive owner/repo matching.
In `@packages/cli/src/recommend-sources.ts`:
- Around line 5-9: The SkillRepoSource interface defined in
packages/cli/src/recommend-sources.ts is duplicating an existing type from the
shared core package. Remove the local interface definition (lines 5-9) and
instead import SkillRepoSource from the core package to maintain a single source
of truth and prevent type drift between the CLI and core implementations.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a6e8c35b-23d8-4429-8b08-8652d91cbdbf
📒 Files selected for processing (10)
packages/cli/README.mdpackages/cli/src/__tests__/recommend-sources.test.tspackages/cli/src/commands/recommend.tspackages/cli/src/recommend-sources.tspackages/core/README.mdpackages/core/src/context/__tests__/detector.test.tspackages/core/src/context/detector.tspackages/core/src/recommend/__tests__/engine.test.tspackages/core/src/recommend/fetcher.tspackages/core/src/recommend/types.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
skillkit recommend --update.Validation
Summary by CodeRabbit
skillkit recommend --updateto refresh the recommendation index from built-in sources plus configured GitHub taps, with automatic deduplication.skillkit recommend --updatebehavior and refresh expectations.