ztimson/ai-agents
1
0
Fork 0
generated from ztimson/template
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

review-labels #14

Merged
ztimson merged 5 commits from review-labels into master 2026-01-14 14:06:21 -05:00
Conversation 2 Commits 5 Files Changed 6 +15 -7
6 changed files with 15 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit 677f84c97a - Show all commits

4
.github/pull_request_template.md vendored
View File

@@ -9,7 +9,5 @@
## Checklist
<!-- Complete after creating PR -->
- [ ] Linked issues
- [ ] Reviewed changes
- [ ] Reviewed changes (or use `Review/AI` label)
- [ ] Updated comments/documentation

2
.github/workflows/code-review.yml vendored
View File

@@ -2,7 +2,7 @@ name: Code review
on:
pull_request:
types: [opened, synchronize, reopened]
types: [opened, synchronize, reopened, labeled]
jobs:
review:

1
.github/workflows/ticket-refinement.yml vendored
View File

@@ -3,6 +3,7 @@ name: Ticket refinement
on:
issues:
types: [labeled]
jobs:
format:
runs-on: ubuntu-latest

2
package.json
View File

@@ -1,6 +1,6 @@
{
"name": "@ztimson/ai-agents",
"version": "0.1.1",
"version": "0.1.2",
"description": "AI agents",
"keywords": ["ai", "review"],
"author": "ztimson",

3
src/refine.mjs
View File

@@ -181,8 +181,9 @@ Output ONLY markdown. No explanations, labels, or extra formatting.`});
const hasDuplicates = (await ai.language.ask(`ID: ${issueData.id}\nTitle: ${title}\n\`\`\`markdown\n${body}\n\`\`\``, {
system: `Your job is to identify duplicates. Respond ONLY with the duplicate's ID number or "NONE" if no match exists\n\n${dupes}`
}))?.pop()?.content;
// Handle duplicates
if(hasDuplicates && hasDuplicates !== 'NONE' && (dupeId = dupeIds.find(id => id == hasDuplicates.trim()))) {
if(hasDuplicates && !hasDuplicates.toUpperCase().includes('NONE') && (dupeId = dupeIds.find(id => id == hasDuplicates.trim())) != null && dupeId != issueData.id) {
ztimson marked this conversation as resolved
assistant commented 2026-01-14 13:47:40 -05:00
Review
Copy Link

Improved duplicate detection logic, but the condition is overly complex. Consider simplifying: dupeId = dupeIds.find(id => id == hasDuplicates.trim()); if(hasDuplicates && !hasDuplicates.toUpperCase().includes('NONE') && dupeId != null && dupeId != issueData.id) could be clearer with early returns or better variable naming.

Improved duplicate detection logic, but the condition is overly complex. Consider simplifying: `dupeId = dupeIds.find(id => id == hasDuplicates.trim()); if(hasDuplicates && !hasDuplicates.toUpperCase().includes('NONE') && dupeId != null && dupeId != issueData.id)` could be clearer with early returns or better variable naming.
assistant commented 2026-01-14 13:47:40 -05:00
Review
Copy Link

Potential type coercion bug: Using == instead of === in dupeIds.find(id => id == hasDuplicates.trim()) and dupeId != issueData.id may cause unexpected behavior if IDs are sometimes strings and sometimes numbers. Use strict equality (=== and !==) for more predictable comparisons.

Potential type coercion bug: Using `==` instead of `===` in `dupeIds.find(id => id == hasDuplicates.trim())` and `dupeId != issueData.id` may cause unexpected behavior if IDs are sometimes strings and sometimes numbers. Use strict equality (`===` and `!==`) for more predictable comparisons.
assistant commented 2026-01-14 13:52:35 -05:00
Review
Copy Link

Potential type coercion bug: Using == instead of === in dupeIds.find(id => id == hasDuplicates.trim()) may cause unexpected behavior if IDs are sometimes strings and sometimes numbers. Use strict equality (===) for more predictable comparisons.

Potential type coercion bug: Using `==` instead of `===` in `dupeIds.find(id => id == hasDuplicates.trim())` may cause unexpected behavior if IDs are sometimes strings and sometimes numbers. Use strict equality (`===`) for more predictable comparisons.
assistant commented 2026-01-14 13:52:35 -05:00
Review
Copy Link

Redundant check: The condition dupeId != issueData.id is unnecessary since the search is limited to 3 results and unlikely to return the current issue. However, if kept, use strict inequality (!==) instead of != for consistency.

Redundant check: The condition `dupeId != issueData.id` is unnecessary since the search is limited to 3 results and unlikely to return the current issue. However, if kept, use strict inequality (`!==`) instead of `!=` for consistency.
await fetch(`${git}/api/v1/repos/${owner}/${repo}/issues/${ticket}/comments`, {
method: 'POST',
headers: {'Authorization': `token ${auth}`, 'Content-Type': 'application/json'},

10
src/review.mjs
View File

@@ -19,12 +19,20 @@ dotenv.config({path: '.env.local', override: true, quiet: true, debug: false});
owner = process.env['GIT_OWNER'],
repo = process.env['GIT_REPO'],
auth = process.env['GIT_TOKEN'],
labelEnabled = process.env['LABEL_ENABLED'] || 'Review/AI',
pr = process.env['PULL_REQUEST'],
host = process.env['AI_HOST'],
model = process.env['AI_MODEL'],
token = process.env['AI_TOKEN'];
console.log(`Reviewing: ${root}\n`);
const info = await fetch(`${git}/api/v1/repos/${owner}/${repo}/pulls/${pr}`)
ztimson marked this conversation as resolved
assistant commented 2026-01-14 13:47:40 -05:00
Review
Copy Link

Missing error handling: The fetch call should handle errors. If the API call fails, info will be undefined and line 30 will throw an error when trying to access info.labels.

Missing error handling: The fetch call should handle errors. If the API call fails, `info` will be undefined and line 30 will throw an error when trying to access `info.labels`.
assistant commented 2026-01-14 13:52:35 -05:00
Review
Copy Link

Missing error handling: The fetch call should handle network errors with a .catch() handler. If the API call fails due to network issues, the promise will reject and crash the script before reaching line 30.

Missing error handling: The fetch call should handle network errors with a .catch() handler. If the API call fails due to network issues, the promise will reject and crash the script before reaching line 30.
.then(async resp => { return resp.ok ? resp.json() : throw new Error(`${resp.status} ${await resp.text()}`); });
ztimson marked this conversation as resolved Outdated
assistant commented 2026-01-14 13:47:40 -05:00
Outdated
Review
Copy Link

Logic error: The condition info.labels?.length > 0 || !info.labels.some(l => l.name === labelEnabled) will always be true when labels exist. This should use && instead of ||. Currently, if labels.length > 0, it skips even if the enabled label is present. The correct logic should be: !info.labels?.some(l => l.name === labelEnabled) or info.labels?.length === 0 || !info.labels.some(l => l.name === labelEnabled)

Logic error: The condition `info.labels?.length > 0 || !info.labels.some(l => l.name === labelEnabled)` will always be true when labels exist. This should use `&&` instead of `||`. Currently, if labels.length > 0, it skips even if the enabled label is present. The correct logic should be: `!info.labels?.some(l => l.name === labelEnabled)` or `info.labels?.length === 0 || !info.labels.some(l => l.name === labelEnabled)`
assistant commented 2026-01-14 13:52:35 -05:00
Outdated
Review
Copy Link

Syntax error: throw new Error(...) cannot be used directly in a ternary expression. This should be wrapped in a block: resp.ok ? resp.json() : (() => { throw new Error(...); })() or use an if-else statement instead.

Syntax error: `throw new Error(...)` cannot be used directly in a ternary expression. This should be wrapped in a block: `resp.ok ? resp.json() : (() => { throw new Error(...); })()` or use an if-else statement instead.
if(info.labels?.length > 0 || !info.labels.some(l => l.name === labelEnabled)) {
ztimson marked this conversation as resolved Outdated
assistant commented 2026-01-14 13:52:35 -05:00
Outdated
Review
Copy Link

Logic error: The condition info.labels?.length > 0 && !info.labels.some(l => l.name === labelEnabled) will skip the review when labels exist but the enabled label is NOT present. This appears backwards - it should run when the label IS present. Consider: if(!info.labels?.some(l => l.name === labelEnabled)) to skip when the label is missing.

Logic error: The condition `info.labels?.length > 0 && !info.labels.some(l => l.name === labelEnabled)` will skip the review when labels exist but the enabled label is NOT present. This appears backwards - it should run when the label IS present. Consider: `if(!info.labels?.some(l => l.name === labelEnabled))` to skip when the label is missing.
console.log('Skipping');
return process.exit();
}
const branch = process.env['GIT_BRANCH'] || await $`cd ${root} && git symbolic-ref refs/remotes/origin/HEAD`;
const comments = [];
const commit = await $`cd ${root} && git log -1 --pretty=format:%H`;
@@ -53,7 +61,7 @@ dotenv.config({path: '.env.local', override: true, quiet: true, debug: false});
...options,
model: [host, model],
path: process.env['path'] || os.tmpdir(),
system: `You are a code reviewer. Analyze the git diff and use the \`recommend\` tool for EACH issue you find. You must call \`recommend\` exactly once for every bug or improvement opportunity directly related to changes. Ignore formatting recommendations. After making all recommendations, provide some concluding remarks about the overall state of the changes.${existingComments}`,
system: `You are a code reviewer. Analyze the git diff and use the \`recommend\` tool for EACH issue you find. You must call \`recommend\` exactly once for every bug or improvement opportunity directly related to changes. Ignore formatting recommendations. After making all recommendations, provide a quick 75 words or less sitrep.${existingComments}`,
ztimson marked this conversation as resolved Outdated
assistant commented 2026-01-14 13:47:40 -05:00
Outdated
Review
Copy Link

Typo in system prompt: 'sitrep' is informal military jargon. Consider using more professional language like 'summary' or 'overview' for better clarity in a code review context.

Typo in system prompt: 'sitrep' is informal military jargon. Consider using more professional language like 'summary' or 'overview' for better clarity in a code review context.
assistant commented 2026-01-14 13:52:35 -05:00
Outdated
Review
Copy Link

Typo in system prompt: 'sitrep' is informal military jargon. Consider using more professional language like 'summary' or 'overview' for better clarity in a code review context.

Typo in system prompt: 'sitrep' is informal military jargon. Consider using more professional language like 'summary' or 'overview' for better clarity in a code review context.
tools: [{
name: 'read_file',
description: 'Read contents of a file',