-
Notifications
You must be signed in to change notification settings - Fork 3
Support local disk read for context files #34
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Added logic to pull configured context files directly from the runner’s workspace in addition to the GitHub API, enabling more comprehensive PR reviews and summaries. Log grouping labels have been updated to clearly separate API and filesystem sources. Walkthrough
Model: o4-mini | Prompt Tokens: 1093 | Completion Tokens: 468 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for sharing your code. AI offers friendly suggestions to enhance your work, but these are optional ideas based on patterns, not absolute rules. Use what resonates with your style and goals. You remain in control; AI is here simply to support your creative journey.
Model: o4-mini | Prompt Tokens: 1811 | Completion Tokens: 2772
| if (config.includeContextFiles.some(pattern => minimatch(filePath, pattern, matchOptions))) { | ||
| if (config.excludeContextFiles.some(pattern => minimatch(filePath, pattern, matchOptions))) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Calling minimatch against each pattern in every iteration can become expensive for large file sets. Precompile your glob patterns into Minimatch instances or create predicate functions once before the loop. For example:
const includeMatchers = config.includeContextFiles.map(p => new Minimatch(p, matchOptions));
const excludeMatchers = config.excludeContextFiles.map(p => new Minimatch(p, matchOptions));
for await (const filePath of walk(process.cwd())) {
if (includeMatchers.some(m => m.match(filePath)) && !excludeMatchers.some(m => m.match(filePath))) {
// ...
}
}There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here's a supportive code review enhanced by AI assistance. These observations offer helpful suggestions and insights, though some are AI-generated predictions rather than definitive conclusions. Please consider what feels most useful for your project. You remain the expert and decision-maker—AI simply provides additional perspective to support your development process.
Model: anthropic--claude-4-sonnet | Prompt Tokens: 2091 | Completion Tokens: 1700
* Add functionality to read context files from the runner's filesystem * Add functionality to read context files from the runner's filesystem
Right now, the AI-assisted actions don’t actually know anything about the file system during a workflow run. They just talk directly to the GitHub REST API to grab whatever they need from the repo. So, you don’t need to use actions/checkout, and even if you do, the action won’t look at the files on disk (at least for now).
Have the action look for the files locally first, and only fall back to grabbing them from the repo blobs if it can’t find them.
This change enhances the context file retrieval functionality by adding a fallback mechanism that reads files directly from the runner's filesystem when the GitHub API method is unavailable or insufficient. The implementation adds filesystem-based file discovery alongside the existing GitHub API approach, providing more reliable access to context files during PR analysis.
Walkthrough
Model: anthropic--claude-4-sonnet | Prompt Tokens: 1336 | Completion Tokens: 151