Skip to content

fix(workflows): improve attempt comments and fix PR body parsing#95

Merged
MarkusNeusinger merged 1 commit intomainfrom
fix/attempt-comments-and-approach
Dec 1, 2025
Merged

fix(workflows): improve attempt comments and fix PR body parsing#95
MarkusNeusinger merged 1 commit intomainfrom
fix/attempt-comments-and-approach

Conversation

@MarkusNeusinger
Copy link
Copy Markdown
Owner

Summary

  • Only post attempt comment to sub-issues if PR was actually created (fixes empty 'Attempt 1/3' comments)
  • Make Technical Approach section more useful: imports, plot function, parameters, config
  • Fix shell escaping in bot-auto-merge.yml (use gh pr view instead of direct variable expansion)

Fixes

- Only post attempt comment if PR was actually created (not on failures)
- Make approach section more technical (imports, plot function, params)
- Fix shell escaping for PR body in bot-auto-merge.yml (use gh pr view)
Copilot AI review requested due to automatic review settings December 1, 2025 20:29
@MarkusNeusinger MarkusNeusinger merged commit 4ae4bd4 into main Dec 1, 2025
2 checks passed
@MarkusNeusinger MarkusNeusinger deleted the fix/attempt-comments-and-approach branch December 1, 2025 20:30
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes two workflow issues: preventing empty attempt comments when PRs aren't created and resolving shell escaping errors in PR body parsing. The changes improve the robustness and informativeness of the automated plot generation workflow.

Key Changes:

  • Added conditional check to only post attempt comments when PR exists (fixes Issue #83)
  • Enhanced Technical Approach section with structured information (imports, plot function, parameters, config)
  • Fixed unsafe PR body variable expansion by using gh pr view command (fixes auto-merge "Permission denied" error)

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
.github/workflows/gen-library-impl.yml Modified attempt comment posting to check PR existence; rewrote Technical Approach extraction to show imports, plot function, parameters, and styling config instead of generic approach bullets
.github/workflows/bot-auto-merge.yml Replaced direct PR body variable expansion with safe gh pr view command to prevent shell escaping issues

You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.

if func_match:
params = func_match.group(1).strip()
# Get first 3 params
param_list = [p.strip().split(':')[0].strip() for p in params.split(',')[:4]]
Copy link

Copilot AI Dec 1, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment says "Get first 3 params" but the code slices [:4], which would get the first 4 parameters. Either update the comment to "Get first 4 params" or change the slice to [:3].

Suggested change
param_list = [p.strip().split(':')[0].strip() for p in params.split(',')[:4]]
param_list = [p.strip().split(':')[0].strip() for p in params.split(',')[:3]]

Copilot uses AI. Check for mistakes.
MarkusNeusinger added a commit that referenced this pull request Dec 1, 2025
## Summary
- Only post attempt comment to sub-issues if PR was actually created
(fixes empty 'Attempt 1/3' comments)
- Make Technical Approach section more useful: imports, plot function,
parameters, config
- Fix shell escaping in bot-auto-merge.yml (use `gh pr view` instead of
direct variable expansion)

## Fixes
- Issue #83: Duplicate 'Attempt 1/3' with first being empty
- Auto-merge workflow failing with 'Permission denied'
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants