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

Added ticket refinement bot #4

Merged
ztimson merged 4 commits from refinement into master 2025-12-30 15:38:49 -05:00
Conversation 5 Commits 4 Files Changed 5 +3 -3
2 changed files with 3 additions and 3 deletions
Download Patch File Download Diff File Expand all files Collapse all files
Showing only changes of commit af09bd0f53 - Show all commits

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

@@ -14,7 +14,7 @@ jobs:
git checkout ${{ github.event.repository.default_branch }}
- name: Run AI Formatter
ztimson marked this conversation as resolved
assistant commented 2025-12-30 13:50:58 -05:00
Review
Copy Link

Path mismatch: The workflow references .github/issue_templates/ai-refinement.md but the actual file is located at .github/issue_template/ai-refinement.md (singular "template" not "templates"). This will cause the workflow to fail when trying to read the template file.

Path mismatch: The workflow references `.github/issue_templates/ai-refinement.md` but the actual file is located at `.github/issue_template/ai-refinement.md` (singular "template" not "templates"). This will cause the workflow to fail when trying to read the template file.
assistant commented 2025-12-30 13:50:58 -05:00
Review
Copy Link

Command name mismatch: The workflow runs npx -y @ztimson/ai-agents@latest format but according to package.json, the binary is named "refine", not "format". This should be npx -y @ztimson/ai-agents@latest refine.

Command name mismatch: The workflow runs `npx -y @ztimson/ai-agents@latest format` but according to package.json, the binary is named "refine", not "format". This should be `npx -y @ztimson/ai-agents@latest refine`.
assistant commented 2025-12-30 14:16:46 -05:00
Review
Copy Link

The workflow calls 'format' subcommand but package.json defines 'refine' as the binary name. This mismatch will cause the workflow to fail. Either change the command to 'npx -y @ztimson/ai-agents@latest refine' or update package.json to use 'format' as the binary name.

The workflow calls 'format' subcommand but package.json defines 'refine' as the binary name. This mismatch will cause the workflow to fail. Either change the command to 'npx -y @ztimson/ai-agents@latest refine' or update package.json to use 'format' as the binary name.
run: npx -y @ztimson/ai-agents@latest format .github/issue_template/ai-refinement.md
run: npx -y @ztimson/ai-agents@latest refine .github/issue_template/ai-refinement.md
env:
AI_HOST: anthropic
ztimson marked this conversation as resolved
assistant commented 2025-12-30 13:50:58 -05:00
Review
Copy Link

Invalid model name: claude-sonnet-4-5 is not a valid Anthropic model name. It should likely be claude-sonnet-4 or claude-3-5-sonnet-20241022 (or similar valid Anthropic model identifier).

Invalid model name: `claude-sonnet-4-5` is not a valid Anthropic model name. It should likely be `claude-sonnet-4` or `claude-3-5-sonnet-20241022` (or similar valid Anthropic model identifier).
assistant commented 2025-12-30 15:23:56 -05:00
Review
Copy Link

The AI_MODEL value 'claude-sonnet-4-5' appears incorrect. Claude model names typically use format like 'claude-sonnet-4-20250514' or 'claude-3-5-sonnet-20241022'. Verify this is a valid model identifier.

The AI_MODEL value 'claude-sonnet-4-5' appears incorrect. Claude model names typically use format like 'claude-sonnet-4-20250514' or 'claude-3-5-sonnet-20241022'. Verify this is a valid model identifier.
AI_MODEL: claude-sonnet-4-5
ztimson marked this conversation as resolved
assistant commented 2025-12-30 15:19:09 -05:00
Review
Copy Link

Hardcoded AI model "claude-sonnet-4-5" appears to be incorrect. The correct model name should be "claude-sonnet-4" or "claude-3-5-sonnet-20240620" based on Anthropic's naming conventions. This will cause API errors.

Hardcoded AI model "claude-sonnet-4-5" appears to be incorrect. The correct model name should be "claude-sonnet-4" or "claude-3-5-sonnet-20240620" based on Anthropic's naming conventions. This will cause API errors.

4
src/refine.mjs
View File

@@ -12,7 +12,7 @@ dotenv.config({path: '.env.local', override: true, quiet: true});
(async () => {
assistant commented 2025-12-30 15:23:57 -05:00
Review
Copy Link

The entire script is wrapped in an async IIFE without proper error handling. If any unhandled error occurs, it will cause an unhandled promise rejection. Add a .catch() block at the end to handle errors gracefully.

The entire script is wrapped in an async IIFE without proper error handling. If any unhandled error occurs, it will cause an unhandled promise rejection. Add a .catch() block at the end to handle errors gracefully.
let p = process.argv[process.argv.length - 1];
ztimson marked this conversation as resolved
assistant commented 2025-12-30 13:50:58 -05:00
Review
Copy Link

Wrong script name in condition: The check if(p === 'review' || p.endsWith('review.mjs')) should be if(p === 'refine' || p.endsWith('refine.mjs')) since this is the refine.mjs script, not review.mjs.

Wrong script name in condition: The check `if(p === 'review' || p.endsWith('review.mjs'))` should be `if(p === 'refine' || p.endsWith('refine.mjs'))` since this is the refine.mjs script, not review.mjs.
if(p === 'refine' || p.endsWith('refine.mjs')) p = null;
ztimson marked this conversation as resolved
assistant commented 2025-12-30 14:16:46 -05:00
Review
Copy Link

The path detection regex '/(\/|[A-Z]:)/.test(p)' checks if the path is absolute, but the logic is inverted - if it's NOT absolute, it joins with cwd. However, this doesn't handle edge cases like './' or '../' relative paths correctly, which would be treated as absolute due to the '/' character.

The path detection regex '/(\\/|[A-Z]:)/.test(p)' checks if the path is absolute, but the logic is inverted - if it's NOT absolute, it joins with cwd. However, this doesn't handle edge cases like './' or '../' relative paths correctly, which would be treated as absolute due to the '/' character.
assistant commented 2025-12-30 15:23:56 -05:00
Review
Copy Link

Path validation regex uses 'm' flag unnecessarily. The multiline flag isn't needed for this single-line path check. Also, the regex doesn't handle relative paths starting with './' or '../' which are valid.

Path validation regex uses 'm' flag unnecessarily. The multiline flag isn't needed for this single-line path check. Also, the regex doesn't handle relative paths starting with './' or '../' which are valid.
if(!/(\/|[A-Z]:)/.test(p)) p = path.join(process.cwd(), p);
if(!/^(\/|[A-Z]:)/m.test(p)) p = path.join(process.cwd(), p);
if(!p || !fs.existsSync(p)) throw new Error('Please provide a template');
assistant commented 2025-12-30 15:19:09 -05:00
Review
Copy Link

Missing validation for required environment variables. If any of git, owner, repo, auth, ticket, host, model, or token are undefined, the script will fail with unclear error messages. Add validation and provide helpful error messages for missing configuration.

Missing validation for required environment variables. If any of git, owner, repo, auth, ticket, host, model, or token are undefined, the script will fail with unclear error messages. Add validation and provide helpful error messages for missing configuration.
@@ -83,7 +83,7 @@ ${template.trim()}
Output ONLY the formatted ticket, no explanation.`
})
const messages = await ai.language.ask(`Title: ${issueData.title}\n\nDescription:\n${issueData.body || 'No description provided'}`).catch(() => []);;
const messages = await ai.language.ask(`Title: ${issueData.title}\n\nDescription:\n${issueData.body || 'No description provided'}`).catch(() => []);
ztimson marked this conversation as resolved Outdated
assistant commented 2025-12-30 13:50:58 -05:00
Outdated
Review
Copy Link

No error handling for AI call: The ai.language.ask() call has no try-catch block. If the AI service fails, times out, or returns an unexpected format, the script will crash without helpful error messages.

No error handling for AI call: The `ai.language.ask()` call has no try-catch block. If the AI service fails, times out, or returns an unexpected format, the script will crash without helpful error messages.
assistant commented 2025-12-30 14:16:46 -05:00
Outdated
Review
Copy Link

Double semicolon syntax error. Remove one semicolon from '.catch(() => []);;'

Double semicolon syntax error. Remove one semicolon from '.catch(() => []);;'
assistant commented 2025-12-30 15:19:09 -05:00
Review
Copy Link

Error handling with .catch(() => []) silently swallows all errors from the AI request. This makes debugging difficult. Consider logging the error or providing more context about what went wrong.

Error handling with `.catch(() => [])` silently swallows all errors from the AI request. This makes debugging difficult. Consider logging the error or providing more context about what went wrong.
assistant commented 2025-12-30 15:23:56 -05:00
Review
Copy Link

Error handling with .catch(() => []) silently swallows all errors, making debugging difficult. At minimum, log the error before returning an empty array, or let it propagate for better error visibility.

Error handling with .catch(() => []) silently swallows all errors, making debugging difficult. At minimum, log the error before returning an empty array, or let it propagate for better error visibility.
const content = messages?.pop()?.content;
ztimson marked this conversation as resolved Outdated
assistant commented 2025-12-30 13:50:58 -05:00
Outdated
Review
Copy Link

Unsafe array access: messages.pop() assumes the array is non-empty. If the AI returns an empty response, this will throw an error when accessing .content. Add a check to ensure messages exist.

Unsafe array access: `messages.pop()` assumes the array is non-empty. If the AI returns an empty response, this will throw an error when accessing `.content`. Add a check to ensure messages exist.
if(!content) {
console.log('Invalid response from AI');
ztimson marked this conversation as resolved Outdated
assistant commented 2025-12-30 13:50:58 -05:00
Outdated
Review
Copy Link

Potential runtime error: type[0].toUpperCase() will throw if type is an empty string. The fallback 'Unassigned' is set earlier, but if the regex extracts an empty string, this will fail. Add a check: type && type.length > 0 before accessing type[0].

Potential runtime error: `type[0].toUpperCase()` will throw if `type` is an empty string. The fallback 'Unassigned' is set earlier, but if the regex extracts an empty string, this will fail. Add a check: `type && type.length > 0` before accessing `type[0]`.