Strengthen self-improvement decision support#35
Conversation
Review Summary by QodoStrengthen self-improvement decision support with policy detection
WalkthroughsDescription• Detect security policies by checking standard SECURITY.md locations • Generate decision-oriented issue content with Adopt/Reject/Defer suggestions • Create standalone decision-log artifact from scheduled workflow • Update conductor track notes reflecting strengthened automation Diagramflowchart LR
A["gather-repo-data.js"] -->|"check SECURITY.md locations"| B["hasPublishedSecurityPolicy"]
A -->|"fetch repo metadata"| C["getRepoMetadata"]
C -->|"include security policy flag"| D["repo-data.json"]
D -->|"process decisions"| E["render-self-improvement-issue.js"]
E -->|"build local/upstream decisions"| F["Decision Items"]
F -->|"format with reasons"| G["self-improvement-issue.md"]
F -->|"standalone log"| H["self-improvement-decisions.md"]
G -->|"upload artifact"| I["Workflow Artifacts"]
H -->|"upload artifact"| I
File Changes1. scripts/gather-repo-data.js
|
Code Review by Qodo
1. File check bypasses retries
|
Summary of ChangesHello, 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 enhances the repository's self-improvement automation by improving how security policies are detected and by introducing automated decision support for pull requests. The changes aim to provide maintainers with clearer, actionable insights and pre-analyzed suggestions, thereby reducing manual effort in evaluating potential improvements and ensuring a more robust and efficient review cycle for both local and upstream changes. 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. Changelog
Ignored Files
Activity
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
|
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThe pull request enhances the self-improvement workflow by introducing decision log generation and policy detection. It updates the GitHub workflow to produce an additional artifact containing structured decision recommendations, adds policy detection utilities to repository metadata gathering, and implements decision formatting functions to render adoption guidance in both the issue body and a separate log file. Changes
Sequence DiagramsequenceDiagram
actor Scheduler as Scheduled Workflow
participant GatherScript as gather-repo-data.js
participant RepoAPI as GitHub API
participant RenderScript as render-self-improvement-issue.js
participant FileSystem as File System
Scheduler->>GatherScript: Fetch repository metadata
par Concurrent Requests
GatherScript->>RepoAPI: Get repo data
GatherScript->>RepoAPI: Check for security policy files
end
RepoAPI-->>GatherScript: Repository data + policy status
GatherScript-->>Scheduler: Enhanced metadata with has_security_policy
Scheduler->>RenderScript: Generate decision content from PRs
RenderScript->>RenderScript: Build local decisions<br/>(format, categorize PRs)
RenderScript->>RenderScript: Build upstream decisions<br/>(format, categorize PRs)
RenderScript->>FileSystem: Write primary issue.md<br/>(with decision rubric & support sections)
RenderScript->>FileSystem: Write decision log<br/>(standalone decision-oriented artifact)
FileSystem-->>Scheduler: Artifacts created
Scheduler->>Scheduler: Upload artifacts to workflow
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
There was a problem hiding this comment.
Code Review
This pull request enhances the self-improvement automation by improving security policy detection, generating decision-oriented issue content, and creating a decision log artifact. The changes correctly detect SECURITY.md files and make API calls more efficient. The rendering script now uses heuristics to suggest 'Adopt/Reject/Defer' decisions for pull requests, which are then included in the generated issue and a new decision log file. My review includes suggestions to improve the robustness of API calls in the data gathering script and to enhance the maintainability of the decision-making logic in the rendering script.
| async function repoFileExists(repo, filePath) { | ||
| const response = await fetch(`${GITHUB_API}/repos/${repo}/contents/${filePath}`, { | ||
| headers: { | ||
| Accept: 'application/vnd.github.v3+json', | ||
| 'User-Agent': 'humanizer-self-improvement-bot', | ||
| }, | ||
| }); | ||
|
|
||
| if (response.status === 404) { | ||
| return false; | ||
| } | ||
|
|
||
| if (!response.ok) { | ||
| throw new Error(`Failed to check ${filePath} in ${repo}: ${response.status}`); | ||
| } | ||
|
|
||
| return true; | ||
| } |
There was a problem hiding this comment.
The repoFileExists function can be improved for efficiency, robustness, and to avoid rate-limiting issues.
- Efficiency: It's using a
GETrequest to check for file existence. AHEADrequest is more efficient as it doesn't transfer the file content, only the headers. - Rate Limiting: The function makes unauthenticated requests to the GitHub API, which are subject to very strict rate limits (60 requests/hour). For a bot, it's crucial to use a
GITHUB_TOKEN(e.g., fromprocess.env.GITHUB_TOKEN) to increase this limit. - Robustness: This function lacks the retry logic present in
fetchGitHub. Network requests can be flaky, and retries are important for a reliable script.
I've suggested a change to use HEAD and include an Authorization header. You should also consider adding retry logic for consistency with fetchGitHub.
async function repoFileExists(repo, filePath) {
const response = await fetch(`${GITHUB_API}/repos/${repo}/contents/${filePath}`, {
method: 'HEAD',
headers: {
Accept: 'application/vnd.github.v3+json',
'User-Agent': 'humanizer-self-improvement-bot',
...(process.env.GITHUB_TOKEN && { Authorization: `token ${process.env.GITHUB_TOKEN}` }),
},
});
if (response.status === 404) {
return false;
}
if (!response.ok) {
throw new Error(`Failed to check ${filePath} in ${repo}: ${response.status}`);
}
return true;
}| function buildLocalDecisions(localPrs) { | ||
| return localPrs.slice(0, 10).map((pr) => { | ||
| const lowerTitle = pr.title.toLowerCase(); | ||
|
|
||
| if (lowerTitle.includes('@changesets/cli')) { | ||
| return { | ||
| scope: 'local', | ||
| number: pr.number, | ||
| title: pr.title, | ||
| decision: 'reject', | ||
| reason: | ||
| 'Changesets is no longer part of the repo release model. This skill-source repo ships artifacts through GitHub, not package releases.', | ||
| }; | ||
| } | ||
|
|
||
| if ( | ||
| lowerTitle.includes('actions/upload-artifact') || | ||
| lowerTitle.includes('create-issue-from-file') | ||
| ) { | ||
| return { | ||
| scope: 'local', | ||
| number: pr.number, | ||
| title: pr.title, | ||
| decision: 'adopt', | ||
| reason: | ||
| 'Workflow dependency updates match the current automation direction and should be merged after the scheduled job passes.', | ||
| }; | ||
| } | ||
|
|
||
| if ( | ||
| lowerTitle.includes('@types/node') || | ||
| lowerTitle.includes('lint-staged') || | ||
| lowerTitle.includes('eslint') | ||
| ) { | ||
| return { | ||
| scope: 'local', | ||
| number: pr.number, | ||
| title: pr.title, | ||
| decision: 'adopt', | ||
| reason: | ||
| 'Maintainer-tooling updates fit the repo contract and should be taken when the local lint, validate, and test gates remain green.', | ||
| }; | ||
| } | ||
|
|
||
| return { | ||
| scope: 'local', | ||
| number: pr.number, | ||
| title: pr.title, | ||
| decision: 'defer', | ||
| reason: 'No repo-specific automation rule exists for this PR yet. Review manually.', | ||
| }; | ||
| }); | ||
| } |
There was a problem hiding this comment.
The buildLocalDecisions function (and buildUpstreamDecisions, which follows the same pattern) uses a series of if statements to classify PRs based on their titles. This approach can become difficult to maintain and extend as more rules are added.
Consider refactoring this logic to be data-driven. You could define the rules in an array of objects, where each object contains matching criteria (e.g., keywords) and the corresponding decision details. A generic function could then iterate through these rules to process the PRs.
This would make the code more declarative, reduce duplication, and make it easier to add, remove, or modify rules in the future—potentially from a separate configuration file.
Here's a conceptual example of how the rules could be structured:
const localDecisionRules = [
{
keywords: ['@changesets/cli'],
decision: 'reject',
reason: 'Changesets is no longer part of the repo release model...'
},
{
keywords: ['actions/upload-artifact', 'create-issue-from-file'],
decision: 'adopt',
reason: 'Workflow dependency updates match the current automation direction...'
},
// ... more rules
];There was a problem hiding this comment.
Pull request overview
Strengthens the weekly self-improvement automation by improving SECURITY.md detection, adding “Adopt/Reject/Defer” decision-oriented output, and persisting a standalone decision-log artifact for maintainers to finalize in the active conductor track.
Changes:
- Detect published security policies by probing standard
SECURITY.mdlocations during repo data gathering. - Generate decision-support sections (rubric + suggested Adopt/Reject/Defer items) and emit a separate decision-log markdown file.
- Upload the decision-log as a workflow artifact and update the active conductor track notes to reflect the stronger (but still human-finalized) loop.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| scripts/render-self-improvement-issue.js | Adds decision rubric + suggested decisions and writes an additional decision-log markdown output. |
| scripts/gather-repo-data.js | Changes security policy detection to check for SECURITY.md in standard locations. |
| conductor/tracks/repo-self-improvement_20260303/spec.md | Updates track notes to reflect improved self-improvement workflow outputs. |
| conductor/tracks/repo-self-improvement_20260303/plan.md | Marks automation tasks complete and documents current state (decision log artifact + maintainer finalization). |
| .github/workflows/self-improvement.yml | Updates step name, uploads the decision log artifact, and tweaks run notices accordingly. |
You can also share your feedback on Copilot code review. Take the survey.
| const decisionsPath = outputPath.replace( | ||
| /self-improvement-issue\.md$/, | ||
| 'self-improvement-decisions.md' | ||
| ); |
| async function repoFileExists(repo, filePath) { | ||
| const response = await fetch(`${GITHUB_API}/repos/${repo}/contents/${filePath}`, { | ||
| headers: { | ||
| Accept: 'application/vnd.github.v3+json', | ||
| 'User-Agent': 'humanizer-self-improvement-bot', | ||
| }, | ||
| }); | ||
|
|
||
| if (response.status === 404) { | ||
| return false; | ||
| } | ||
|
|
||
| if (!response.ok) { | ||
| throw new Error(`Failed to check ${filePath} in ${repo}: ${response.status}`); | ||
| } | ||
|
|
||
| return true; | ||
| } |
| async function repoFileExists(repo, filePath) { | ||
| const response = await fetch(`${GITHUB_API}/repos/${repo}/contents/${filePath}`, { | ||
| headers: { | ||
| Accept: 'application/vnd.github.v3+json', | ||
| 'User-Agent': 'humanizer-self-improvement-bot', | ||
| }, | ||
| }); | ||
|
|
||
| if (response.status === 404) { | ||
| return false; | ||
| } | ||
|
|
||
| if (!response.ok) { | ||
| throw new Error(`Failed to check ${filePath} in ${repo}: ${response.status}`); | ||
| } | ||
|
|
||
| return true; | ||
| } |
There was a problem hiding this comment.
1. File check bypasses retries 🐞 Bug ⛯ Reliability
repoFileExists() performs a raw fetch() without the retry/rate-limit handling used by fetchGitHub(), so transient GitHub errors (or throttling) will throw and abort getRepoMetadata() and the whole gatherer run.
Agent Prompt
### Issue description
`repoFileExists()` uses a raw `fetch()` and throws on any non-404 error without retries/rate-limit handling, which can fail the entire scheduled intelligence gather.
### Issue Context
The codebase already has `fetchGitHub()` with retries and rate-limit handling, but the new SECURITY.md probe bypasses it.
### Fix Focus Areas
- scripts/gather-repo-data.js[27-90]
- scripts/gather-repo-data.js[149-167]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| // GitHub API base URL | ||
| const GITHUB_API = 'https://api.github.com'; | ||
| const SECURITY_POLICY_CANDIDATES = ['SECURITY.md', '.github/SECURITY.md', 'docs/SECURITY.md']; | ||
|
|
||
| /** | ||
| * Fetch data from GitHub API with rate limit handling |
There was a problem hiding this comment.
2. Missing github auth header 🐞 Bug ⛯ Reliability
The new SECURITY.md probing adds extra GitHub API calls but none of the API requests include an Authorization header (e.g., GITHUB_TOKEN), increasing the likelihood of throttling/403 failures in the scheduled workflow.
Agent Prompt
### Issue description
All GitHub API requests are anonymous; the PR adds more API calls for SECURITY.md detection, which increases the chance of 403/throttling and failed scheduled runs.
### Issue Context
`fetchGitHub()` and `repoFileExists()` only set `Accept` + `User-Agent` headers.
### Fix Focus Areas
- scripts/gather-repo-data.js[27-75]
- .github/workflows/self-improvement.yml[45-49]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
| const outputPath = | ||
| process.argv[3] || path.join(REPO_ROOT, '.github', 'generated', 'self-improvement-issue.md'); | ||
| const decisionsPath = outputPath.replace( | ||
| /self-improvement-issue\.md$/, | ||
| 'self-improvement-decisions.md' | ||
| ); |
There was a problem hiding this comment.
3. Decision log may overwrite 🐞 Bug ✓ Correctness
If scripts/render-self-improvement-issue.js is invoked with a custom outputPath that does not end in self-improvement-issue.md, decisionsPath will equal outputPath and the decision log write will overwrite the issue body.
Agent Prompt
### Issue description
`decisionsPath` derivation can collapse to `outputPath`, causing the decision log to overwrite the issue markdown when a non-standard output filename is used.
### Issue Context
The script supports `process.argv[3]` for output path customization.
### Fix Focus Areas
- scripts/render-self-improvement-issue.js[157-166]
- scripts/render-self-improvement-issue.js[242-246]
ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ae6d27d5b5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const decisionsPath = outputPath.replace( | ||
| /self-improvement-issue\.md$/, | ||
| 'self-improvement-decisions.md' |
There was a problem hiding this comment.
Derive decision-log path without assuming output filename
If process.argv[3] is any path that does not end with self-improvement-issue.md, this replace(...) call returns the original outputPath, so the later fs.writeFileSync(decisionsPath, ...) overwrites the issue body file with the decision log. Running node scripts/render-self-improvement-issue.js <data.json> /tmp/custom.md reproduces this (only the decision log remains), so custom script invocations silently lose the generated issue content.
Useful? React with 👍 / 👎.
ae6d27d to
2de87bb
Compare
Summary
Verification
Summary by CodeRabbit